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.

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

Move common routines to draw-common.c #98

Merged
merged 1 commit into from
Feb 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ libtwin.a_files-y = \
src/pattern.c \
src/spline.c \
src/work.c \
src/draw-common.c \
src/hull.c \
src/icon.c \
src/pixmap.c \
Expand All @@ -61,7 +62,7 @@ libtwin.a_files-$(CONFIG_LOGGING) += src/log.c
libtwin.a_files-$(CONFIG_CURSOR) += src/cursor.c

# Renderer
libtwin.a_files-$(CONFIG_RENDERER_BUILTIN) += src/draw.c
libtwin.a_files-$(CONFIG_RENDERER_BUILTIN) += src/draw-builtin.c
libtwin.a_files-$(CONFIG_RENDERER_PIXMAN) += src/draw-pixman.c
libtwin.a_cflags-$(CONFIG_RENDERER_PIXMAN) += $(shell pkg-config --cflags pixman-1)
ifeq ($(CONFIG_RENDERER_PIXMAN), y)
Expand Down
6 changes: 5 additions & 1 deletion include/twin.h
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,7 @@ twin_pixmap_t *twin_make_cursor(int *hx, int *hy);
void twin_dispatch(twin_context_t *ctx);

/*
* draw.c
* draw-*.c
*/

void twin_composite(twin_pixmap_t *dst,
Expand All @@ -648,6 +648,10 @@ void twin_fill(twin_pixmap_t *dst,
twin_coord_t right,
twin_coord_t bottom);

/*
* draw-common.c
*/

void twin_premultiply_alpha(twin_pixmap_t *px);

/*
Expand Down
43 changes: 0 additions & 43 deletions src/draw.c → src/draw-builtin.c
Original file line number Diff line number Diff line change
Expand Up @@ -681,49 +681,6 @@ void twin_composite(twin_pixmap_t *dst,
msk_y, operator, width, height);
}

static twin_argb32_t _twin_apply_alpha(twin_argb32_t v)
{
uint16_t t1, t2, t3;
twin_a8_t alpha = twin_get_8(v,
#if __BYTE_ORDER == __BIG_ENDIAN
0
#else
24
#endif
);

/* clear RGB data if alpha is zero */
if (!alpha)
return 0;

#if __BYTE_ORDER == __BIG_ENDIAN
/* twin needs ARGB format */
return alpha << 24 | twin_int_mult(twin_get_8(v, 24), alpha, t1) << 16 |
twin_int_mult(twin_get_8(v, 16), alpha, t2) << 8 |
twin_int_mult(twin_get_8(v, 8), alpha, t3) << 0;
#else
return alpha << 24 | twin_int_mult(twin_get_8(v, 0), alpha, t1) << 16 |
twin_int_mult(twin_get_8(v, 8), alpha, t2) << 8 |
twin_int_mult(twin_get_8(v, 16), alpha, t3) << 0;
#endif
}

void twin_premultiply_alpha(twin_pixmap_t *px)
{
int x, y;
twin_pointer_t p;

if (px->format != TWIN_ARGB32)
return;

for (y = 0; y < px->height; y++) {
p.b = px->p.b + y * px->stride;

for (x = 0; x < px->width; x++)
p.argb32[x] = _twin_apply_alpha(p.argb32[x]);
}
}

/*
* array primary index is OVER SOURCE
* array secondary index is ARGB32 RGB16 A8
Expand Down
48 changes: 48 additions & 0 deletions src/draw-common.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* Twin - A Tiny Window System
* Copyright (c) 2004 Keith Packard <[email protected]>
* Copyright (c) 2024 National Cheng Kung University, Taiwan
* All rights reserved.
*/

#include "twin_private.h"

static twin_argb32_t _twin_apply_alpha(twin_argb32_t v)
{
uint16_t t1, t2, t3;
twin_a8_t alpha = twin_get_8(v,
#if __BYTE_ORDER == __BIG_ENDIAN
0
#else
24
#endif
);

/* clear RGB data if alpha is zero */
if (!alpha)
return 0;

#if __BYTE_ORDER == __BIG_ENDIAN
/* twin needs ARGB format */
return alpha << 24 | twin_int_mult(twin_get_8(v, 24), alpha, t1) << 16 |
twin_int_mult(twin_get_8(v, 16), alpha, t2) << 8 |
twin_int_mult(twin_get_8(v, 8), alpha, t3) << 0;
#else
return alpha << 24 | twin_int_mult(twin_get_8(v, 0), alpha, t1) << 16 |
twin_int_mult(twin_get_8(v, 8), alpha, t2) << 8 |
twin_int_mult(twin_get_8(v, 16), alpha, t3) << 0;
#endif
Comment on lines +10 to +34
Copy link

@bito-code-review bito-code-review bot Feb 3, 2025

Choose a reason for hiding this comment

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

Consider refactoring byte order handling logic

Consider extracting the byte order specific logic into separate helper functions. The current implementation duplicates logic between big and little endian cases in _twin_apply_alpha.

Code suggestion
Check the AI-generated fix before applying
Suggested change
static twin_argb32_t _twin_apply_alpha(twin_argb32_t v)
{
uint16_t t1, t2, t3;
twin_a8_t alpha = twin_get_8(v,
#if __BYTE_ORDER == __BIG_ENDIAN
0
#else
24
#endif
);
/* clear RGB data if alpha is zero */
if (!alpha)
return 0;
#if __BYTE_ORDER == __BIG_ENDIAN
/* twin needs ARGB format */
return alpha << 24 | twin_int_mult(twin_get_8(v, 24), alpha, t1) << 16 |
twin_int_mult(twin_get_8(v, 16), alpha, t2) << 8 |
twin_int_mult(twin_get_8(v, 8), alpha, t3) << 0;
#else
return alpha << 24 | twin_int_mult(twin_get_8(v, 0), alpha, t1) << 16 |
twin_int_mult(twin_get_8(v, 8), alpha, t2) << 8 |
twin_int_mult(twin_get_8(v, 16), alpha, t3) << 0;
#endif
static twin_argb32_t _twin_apply_alpha_be(twin_argb32_t v, twin_a8_t alpha) {
uint16_t t1, t2, t3;
return alpha << 24 | twin_int_mult(twin_get_8(v, 24), alpha, t1) << 16 |
twin_int_mult(twin_get_8(v, 16), alpha, t2) << 8 |
twin_int_mult(twin_get_8(v, 8), alpha, t3) << 0;
}
static twin_argb32_t _twin_apply_alpha_le(twin_argb32_t v, twin_a8_t alpha) {
uint16_t t1, t2, t3;
return alpha << 24 | twin_int_mult(twin_get_8(v, 0), alpha, t1) << 16 |
twin_int_mult(twin_get_8(v, 8), alpha, t2) << 8 |
twin_int_mult(twin_get_8(v, 16), alpha, t3) << 0;
}

Code Review Run #332d03


Is this a valid issue, or was it incorrectly flagged by the Agent?

  • it was incorrectly flagged

}

void twin_premultiply_alpha(twin_pixmap_t *px)
{
if (px->format != TWIN_ARGB32)
return;

for (twin_coord_t y = 0; y < px->height; y++) {
twin_pointer_t p = {.b = px->p.b + y * px->stride};

for (twin_coord_t x = 0; x < px->width; x++)
p.argb32[x] = _twin_apply_alpha(p.argb32[x]);
}
}
44 changes: 0 additions & 44 deletions src/draw-pixman.c
Original file line number Diff line number Diff line change
Expand Up @@ -166,47 +166,3 @@ void twin_fill(twin_pixmap_t *_dst,

pixman_image_unref(dst);
}

/* Same function in draw.c */
static twin_argb32_t _twin_apply_alpha(twin_argb32_t v)
{
uint16_t t1, t2, t3;
twin_a8_t alpha = twin_get_8(v,
#if __BYTE_ORDER == __BIG_ENDIAN
0
#else
24
#endif
);

/* clear RGB data if alpha is zero */
if (!alpha)
return 0;

#if __BYTE_ORDER == __BIG_ENDIAN
/* twin needs ARGB format */
return alpha << 24 | twin_int_mult(twin_get_8(v, 24), alpha, t1) << 16 |
twin_int_mult(twin_get_8(v, 16), alpha, t2) << 8 |
twin_int_mult(twin_get_8(v, 8), alpha, t3) << 0;
#else
return alpha << 24 | twin_int_mult(twin_get_8(v, 0), alpha, t1) << 16 |
twin_int_mult(twin_get_8(v, 8), alpha, t2) << 8 |
twin_int_mult(twin_get_8(v, 16), alpha, t3) << 0;
#endif
}

void twin_premultiply_alpha(twin_pixmap_t *px)
{
int x, y;
twin_pointer_t p;

if (px->format != TWIN_ARGB32)
return;

for (y = 0; y < px->height; y++) {
p.b = px->p.b + y * px->stride;

for (x = 0; x < px->width; x++)
p.argb32[x] = _twin_apply_alpha(p.argb32[x]);
}
}