Skip to content

Commit 23f4745

Browse files
committed
Common: fix RGB shifts in LoadPNG/SavePNG
1 parent b49e4de commit 23f4745

3 files changed

Lines changed: 123 additions & 14 deletions

File tree

Common/gfx/bitmapdata.cpp

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,83 @@ bool CopyConvert(uint8_t *dst_buffer, const PixelFormat dst_fmt, const size_t ds
111111
return false;
112112
}
113113

114+
void CopySwapRGBA(const uint8_t *src_buffer, const size_t src_pitch, int src_r_shift, int src_g_shift, int src_b_shift, int src_a_shift,
115+
uint8_t *dst_buffer, const size_t dst_pitch, int dst_r_shift, int dst_g_shift, int dst_b_shift, int dst_a_shift,
116+
const int width, const int height, const PixelFormat px_fmt)
117+
{
118+
const int bpp = PixelFormatToPixelBytes(px_fmt);
119+
if (bpp <= 1)
120+
return; // nothing to swap
121+
122+
switch (bpp)
123+
{
124+
case 2:
125+
{
126+
const uint8_t *src_end = src_buffer + src_pitch * height;
127+
for (; src_buffer < src_end; src_buffer += src_pitch, dst_buffer += dst_pitch)
128+
{
129+
const uint16_t *src_ptr = reinterpret_cast<const uint16_t*>(src_buffer);
130+
uint16_t *dst_ptr = reinterpret_cast<uint16_t*>(dst_buffer);
131+
for (int x = 0; x < width; ++x)
132+
{
133+
uint16_t c = *(src_ptr++);
134+
*(dst_ptr++) =
135+
((c >> src_r_shift) & 0x1F) << dst_r_shift |
136+
((c >> src_g_shift) & 0x3F) << dst_g_shift |
137+
((c >> src_b_shift) & 0x1F) << dst_b_shift;
138+
}
139+
}
140+
}
141+
break;
142+
case 3:
143+
{
144+
const uint8_t *src_ptr = src_buffer;
145+
const uint8_t *src_end = src_buffer + src_pitch * height;
146+
for (uint8_t *dst_ptr = dst_buffer; src_ptr < src_end; src_ptr += src_pitch, dst_ptr += dst_pitch)
147+
{
148+
for (int x = 0; x < width; ++x, src_ptr += 3, dst_ptr += 3)
149+
{
150+
int32_t c = Memory::ReadInt24(src_ptr);
151+
int32_t c2 =
152+
((c >> src_r_shift) & 0xFF) << dst_r_shift |
153+
((c >> src_g_shift) & 0xFF) << dst_g_shift |
154+
((c >> src_b_shift) & 0xFF) << dst_b_shift;
155+
Memory::WriteInt24(dst_ptr, c2);
156+
}
157+
}
158+
}
159+
break;
160+
case 4:
161+
{
162+
const uint8_t *src_end = src_buffer + src_pitch * height;
163+
for (; src_buffer < src_end; src_buffer += src_pitch, dst_buffer += dst_pitch)
164+
{
165+
const uint32_t *src_ptr = reinterpret_cast<const uint32_t*>(src_buffer);
166+
uint32_t *dst_ptr = reinterpret_cast<uint32_t*>(dst_buffer);
167+
for (int x = 0; x < width; ++x)
168+
{
169+
uint32_t c = *(src_ptr++);
170+
*(dst_ptr++) =
171+
((c >> src_r_shift) & 0xFF) << dst_r_shift |
172+
((c >> src_g_shift) & 0xFF) << dst_g_shift |
173+
((c >> src_b_shift) & 0xFF) << dst_b_shift |
174+
((c >> src_a_shift) & 0xFF) << dst_a_shift;
175+
}
176+
}
177+
}
178+
break;
179+
}
180+
}
181+
182+
void CopySwapRGBA(const uint8_t *src_buffer, int src_r_shift, int src_g_shift, int src_b_shift, int src_a_shift,
183+
uint8_t *dst_buffer, int dst_r_shift, int dst_g_shift, int dst_b_shift, int dst_a_shift,
184+
const int width, const int height, const PixelFormat px_fmt)
185+
{
186+
const size_t pitch = GetStrideForPixelFormat(px_fmt, width);
187+
CopySwapRGBA(src_buffer, pitch, src_r_shift, src_g_shift, src_b_shift, src_a_shift,
188+
dst_buffer, pitch, dst_r_shift, dst_g_shift, dst_b_shift, dst_a_shift, width, height, px_fmt);
189+
}
190+
114191
} // namespace PixelOperations
115192

116193
} // namespace Common

Common/gfx/bitmapdata.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,17 @@ namespace PixelOp
223223
// add more common conversions later!
224224
bool CopyConvert(uint8_t *dst_buffer, const PixelFormat dst_fmt, const size_t dst_pitch,
225225
const int width, const int height, const uint8_t *src_buffer, const PixelFormat src_fmt, const size_t src_pitch);
226+
// Copies pixels from source to dest buffer, swapping the RGB components, according
227+
// to the provided RGB shifts. This operation requires that pixel format is kept the same.
228+
void CopySwapRGBA(const uint8_t *src_buffer, const size_t src_pitch, int src_r_shift, int src_g_shift, int src_b_shift, int src_a_shift,
229+
uint8_t *dst_buffer, const size_t dst_pitch, int dst_r_shift, int dst_g_shift, int dst_b_shift, int dst_a_shift,
230+
const int width, const int height, const PixelFormat px_fmt);
231+
// Copies pixels from source to dest buffer, swapping the RGB components, according
232+
// to the provided RGB shifts. This operation requires that pixel format is kept the same.
233+
// The source and destination pitches are calculated from PixelFormat
234+
void CopySwapRGBA(const uint8_t *src_buffer, int src_r_shift, int src_g_shift, int src_b_shift, int src_a_shift,
235+
uint8_t *dst_buffer, int dst_r_shift, int dst_g_shift, int dst_b_shift, int dst_a_shift,
236+
const int width, const int height, const PixelFormat px_fmt);
226237
}
227238

228239
} // namespace Common

Common/gfx/image_png.cpp

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,11 @@ namespace Common
3939
namespace ImageFile
4040
{
4141

42+
#define PNG_SHIFT_R32 (0)
43+
#define PNG_SHIFT_G32 (8)
44+
#define PNG_SHIFT_B32 (16)
45+
#define PNG_SHIFT_A32 (24)
46+
4247
static int STB_IO_read(void *user, char *data, int size)
4348
{
4449
return static_cast<int>(static_cast<IStreamBase*>(user)->Read(data, size));
@@ -89,27 +94,31 @@ PixelBuffer LoadPNG(Stream *in, PixelFormat *src_fmt, RGB *pal)
8994
}
9095

9196
PixelBuffer pxbuf;
92-
if (stb_format == STBI_grey || stb_format == STBI_rgb || stb_format == STBI_rgb_alpha)
97+
if (stb_format == STBI_grey)
9398
{
9499
// FIXME: support PixelBuffer with custom deleter, then we may attach stb buffer here
95100
pxbuf = PixelBuffer(width, height, px_fmt);
96-
// stb_format matches bytes-per-pixel in this case
97-
std::memcpy(pxbuf.GetData(), pixels, width * height * stb_format);
101+
std::memcpy(pxbuf.GetData(), pixels, width * height);
98102
// Set a grayscale palette for gray images
99-
if (stb_format == STBI_grey)
103+
if (pal)
100104
{
101-
if (pal)
105+
for (int i = 0; i < PAL_SIZE; ++i)
102106
{
103-
for (int i = 0; i < PAL_SIZE; ++i)
104-
{
105-
pal[i].r = (uint8_t)i;
106-
pal[i].g = (uint8_t)i;
107-
pal[i].b = (uint8_t)i;
108-
pal[i].a = 0;
109-
}
107+
pal[i].r = (uint8_t)i;
108+
pal[i].g = (uint8_t)i;
109+
pal[i].b = (uint8_t)i;
110+
pal[i].a = 0;
110111
}
111112
}
112113
}
114+
else if (stb_format == STBI_rgb || stb_format == STBI_rgb_alpha)
115+
{
116+
// TODO: what about 16-bit images, are they even possible here?
117+
pxbuf = PixelBuffer(width, height, px_fmt);
118+
PixelOp::CopySwapRGBA(pixels, width * stb_format, PNG_SHIFT_R32, PNG_SHIFT_G32, PNG_SHIFT_B32, PNG_SHIFT_A32,
119+
pxbuf.GetData(), GetStrideForPixelFormat(px_fmt, width), _rgb_r_shift_32, _rgb_g_shift_32, _rgb_b_shift_32, _rgb_a_shift_32,
120+
width, height, px_fmt);
121+
}
113122
else if (stb_format == STBI_grey_alpha)
114123
{
115124
pxbuf = PixelBuffer(width, height, px_fmt);
@@ -137,7 +146,7 @@ PixelBuffer LoadPNG(Stream *in, PixelFormat *src_fmt, RGB *pal)
137146
}
138147

139148
stbi_image_free(pixels);
140-
if (src_fmt)
149+
if (pxbuf && src_fmt)
141150
*src_fmt = px_fmt;
142151
return pxbuf;
143152
}
@@ -147,9 +156,21 @@ bool SavePNG(const BitmapData &bmp, bool skip_alpha, const RGB *pal, Stream *out
147156
// TODO: support skip_alpha
148157
// FIXME: support saving palette!!
149158

159+
PixelBuffer png_buf;
160+
if (bmp.GetBytesPerPixel() > 1)
161+
{
162+
png_buf = PixelBuffer(bmp.GetWidth(), bmp.GetHeight(), bmp.GetFormat());
163+
if (bmp.GetBytesPerPixel() == 2)
164+
PixelOp::CopySwapRGBA(bmp.GetData(), _rgb_r_shift_16, _rgb_g_shift_16, _rgb_b_shift_16, 0,
165+
png_buf.GetData(), 0, 5, 11 /*FIXME!*/, 0, bmp.GetWidth(), bmp.GetHeight(), bmp.GetFormat());
166+
else
167+
PixelOp::CopySwapRGBA(bmp.GetData(), _rgb_r_shift_32, _rgb_g_shift_32, _rgb_b_shift_32, _rgb_a_shift_32,
168+
png_buf.GetData(), PNG_SHIFT_R32, PNG_SHIFT_G32, PNG_SHIFT_B32, PNG_SHIFT_A32, bmp.GetWidth(), bmp.GetHeight(), bmp.GetFormat());
169+
}
170+
150171
// Write a compressed PNG to memory
151172
size_t size = 0;
152-
void *png = tdefl_write_image_to_png_file_in_memory_ex(bmp.GetData(), bmp.GetWidth(), bmp.GetHeight(), bmp.GetBytesPerPixel(),
173+
void *png = tdefl_write_image_to_png_file_in_memory_ex(png_buf ? png_buf.GetData() : bmp.GetData(), bmp.GetWidth(), bmp.GetHeight(), bmp.GetBytesPerPixel(),
153174
&size, Z_DEFAULT_COMPRESSION, MZ_FALSE);
154175

155176
// Write a result PNG from memory to file

0 commit comments

Comments
 (0)