Move common routines to draw-common.c

[weihsinyeh]

Move the common routines twin_premultiply_alpha() and _twin_apply_alpha() in src/draw.c to src/draw-common.c and put the dedicated implementations into src/draw-builtin.c and src/draw-pixman.c, respectively.

Summary by Bito

Reorganization of alpha handling routines by moving twin_premultiply_alpha and _twin_apply_alpha from src/draw.c to new src/draw-common.c file. Draw.c renamed to draw-builtin.c and removed duplicate implementations from draw-pixman.c. Includes updates to Makefile and header files to reflect the structural changes.

Unit tests added: False

Estimated effort to review (1-5, lower is better): 2

[jserv]

Thank @weihsinyeh for contributing!

[bito-code-review]

Code Review Agent Run #332d03

Actionable Suggestions - 1

• src/draw-common.c - 1

  • Consider refactoring byte order handling logic · Line 10-34

Review Details

• Files reviewed - 5 · Commit Range: 7c83fa7..7c83fa7

  • Makefile
  • include/twin.h
  • src/draw-builtin.c
  • src/draw-common.c
  • src/draw-pixman.c
• Files skipped - 0
• Tools

  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful
  • Fb Infer (Static Code Analysis) - ✖︎ Failed

---

AI Code Review powered by

[bito-code-review]

Changelist by Bito

This pull request implements the following key changes.

Key Change	Files Impacted
Feature Improvement - Code Organization and Refactoring	- Makefile - Added draw-common.c and renamed draw.c to draw-builtin.c - twin.h - Updated header comments to reflect new file structure - draw-builtin.c - Removed common alpha handling routines - draw-common.c - Created new file with common alpha handling routines - draw-pixman.c - Removed duplicate alpha handling routines

[bito-code-review]

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