mirror of
https://github.com/libretro/RetroArch.git
synced 2026-11-07 08:36:02 -10:00
scaler/pixconv: fix conv_rgb565_abgr8888 SSE2 byte order
The SSE2 fast path of conv_rgb565_abgr8888 produced ARGB byte order in memory ([b g r a] per pixel), not ABGR ([r g b a]). The scalar fallback (lines 387-396) and the NEON fast path (lines 369-385) both produce the correct ABGR byte order. On x86 hosts with SSE2 the function silently returned a different byte layout than its name promised and than its scalar fallback delivered. Root cause: the SSE2 kernel at lines 358-365 was a literal copy of the conv_rgb565_argb8888 SSE2 kernel at lines 247-255 (the two were byte-for-byte identical). Both pair (b,g) into the low half of each pixel and (r,a) into the high half shifted up 2 bytes, producing [b g r a] byte order which is ARGB uint32 LE. That's correct for argb8888 but wrong for abgr8888. Fix: swap the unpack pairings so (r,g) goes into the low half and (b,a) into the high half shifted up 2 bytes, producing [r g b a] byte order == ABGR uint32 LE. Same instruction count (4 unpacks + 1 shift + 2 ors), same vector width (8 pixels per iteration), no perf change. Verified bit-exact against the scalar fallback for every possible RGB565 input. Brute-force test of all 65,536 16-bit values: before fix: 63,488 of 65,536 outputs differ from scalar (97%) after fix: 0 of 65,536 differ For pure red input 0xF800: scalar: 0xFF0000FF (ABGR uint32, R in low byte) SSE before: 0xFFFF0000 (ARGB uint32, R in second byte) SSE after: 0xFF0000FF (matches scalar) Caller audit: of the production sites that route to pixconv via the SCALER_FMT_ABGR8888 dispatch in scaler.c:126, none are currently observed to hit the RGB565 -> ABGR8888 pair on an x86 host running SSE2. vulkan.c and gl3.c readback paths use ABGR8888 -> BGR24 (different pair). switch_nx_gfx is ARM64 (NEON, not affected). pipewire camera input could in principle hit it if a camera produces RGB565 output, but modern webcams typically produce YUYV / MJPEG. The bug has been hidden by absence of production callers; this fix prevents future contributors from inheriting it.
This commit is contained in:
1 parent
e02161e1f2
commit
940371933f
1 file changed
+13
-9
@@ -347,7 +347,7 @@ void conv_rgb565_abgr8888(void *output_, const void *input_,
|
||||
for (; w < max_width; w += 8)
|
||||
{
|
||||
__m128i res_lo, res_hi;
|
||||
__m128i res_lo_bg, res_hi_bg, res_lo_ra, res_hi_ra;
|
||||
__m128i res_lo_rg, res_hi_rg, res_lo_ba, res_hi_ba;
|
||||
const __m128i in = _mm_loadu_si128((const __m128i*)(input + w));
|
||||
__m128i r = _mm_and_si128(_mm_srli_epi16(in, 1), pix_mask_r);
|
||||
__m128i g = _mm_and_si128(in, pix_mask_g);
|
||||
@@ -355,14 +355,18 @@ void conv_rgb565_abgr8888(void *output_, const void *input_,
|
||||
r = _mm_mulhi_epi16(r, mul16_r);
|
||||
g = _mm_mulhi_epi16(g, mul16_g);
|
||||
b = _mm_mulhi_epi16(b, mul16_b);
|
||||
res_lo_bg = _mm_unpacklo_epi8(b, g);
|
||||
res_hi_bg = _mm_unpackhi_epi8(b, g);
|
||||
res_lo_ra = _mm_unpacklo_epi8(r, a);
|
||||
res_hi_ra = _mm_unpackhi_epi8(r, a);
|
||||
res_lo = _mm_or_si128(res_lo_bg,
|
||||
_mm_slli_si128(res_lo_ra, 2));
|
||||
res_hi = _mm_or_si128(res_hi_bg,
|
||||
_mm_slli_si128(res_hi_ra, 2));
|
||||
/* Output byte order [r g b a] per pixel == ABGR uint32 LE,
|
||||
* matching the scalar fallback below. Pair (r,g) into the
|
||||
* low half and (b,a) into the high half (shifted up 2 bytes),
|
||||
* not (b,g) and (r,a) which would produce ARGB byte order. */
|
||||
res_lo_rg = _mm_unpacklo_epi8(r, g);
|
||||
res_hi_rg = _mm_unpackhi_epi8(r, g);
|
||||
res_lo_ba = _mm_unpacklo_epi8(b, a);
|
||||
res_hi_ba = _mm_unpackhi_epi8(b, a);
|
||||
res_lo = _mm_or_si128(res_lo_rg,
|
||||
_mm_slli_si128(res_lo_ba, 2));
|
||||
res_hi = _mm_or_si128(res_hi_rg,
|
||||
_mm_slli_si128(res_hi_ba, 2));
|
||||
_mm_storeu_si128((__m128i*)(output + w + 0), res_lo);
|
||||
_mm_storeu_si128((__m128i*)(output + w + 4), res_hi);
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user