Skip to content

Commit afaaf6d

Browse files
aras-paras-p
authored
BMP: fix reading 16bpp images (#2808) (#3592)
16 bit per pixel (e.g. 555, 565, 444) formats probably have never worked. One part of the code was assuming 5 bits in the mask, another was shifting by 4 bits, and the 16-bit pixel fetch code was reading just one byte.
1 parent 3e3fba6 commit afaaf6d

File tree

5 files changed

+82
-23
lines changed

5 files changed

+82
-23
lines changed

‎src/bmp.imageio/bmp_pvt.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,8 @@ DibInformationHeader::read_header(Filesystem::IOProxy* fd)
101101
return false;
102102
}
103103

104-
if (size == WINDOWS_V4 || size == WINDOWS_V5 || size == UNDOCHEADER52
104+
if ((size == WINDOWS_V3 && bpp == 16 && compression == 3)
105+
|| size == WINDOWS_V4 || size == WINDOWS_V5 || size == UNDOCHEADER52
105106
|| size == UNDOCHEADER56) {
106107
if (!fread(fd, &red_mask) || !fread(fd, &green_mask)
107108
|| !fread(fd, &blue_mask)) {

‎src/bmp.imageio/bmp_pvt.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,10 @@ class DibInformationHeader {
8585
// in most cases ignored
8686

8787
// added in Version 4 of the format
88-
int32_t red_mask;
89-
int32_t blue_mask;
90-
int32_t green_mask;
91-
int32_t alpha_mask;
88+
int32_t red_mask = 0;
89+
int32_t blue_mask = 0;
90+
int32_t green_mask = 0;
91+
int32_t alpha_mask = 0;
9292
int32_t cs_type; //color space type
9393
int32_t red_x;
9494
int32_t red_y;

‎src/bmp.imageio/bmpinput.cpp

Lines changed: 49 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ class BmpInput final : public ImageInput {
4040
std::vector<bmp_pvt::color_table> m_colortable;
4141
std::vector<unsigned char> fscanline; // temp space: read from file
4242
std::vector<unsigned char> m_uncompressed; // uncompressed palette image
43+
uint32_t m_right_shifts[3];
44+
uint32_t m_bit_counts[3];
4345
bool m_allgray;
4446

4547
void init(void)
@@ -108,6 +110,28 @@ BmpInput::open(const std::string& name, ImageSpec& newspec)
108110
return open(name, newspec, emptyconfig);
109111
}
110112

113+
inline void
114+
calc_shifts(uint32_t mask, uint32_t& count, uint32_t& right)
115+
{
116+
if (mask == 0) {
117+
count = right = 0;
118+
return;
119+
}
120+
121+
uint32_t i;
122+
for (i = 0; i < 32; i++, mask >>= 1) {
123+
if (mask & 1)
124+
break;
125+
}
126+
right = i;
127+
128+
for (i = 0; i < 32; i++, mask >>= 1) {
129+
if (!(mask & 1))
130+
break;
131+
}
132+
count = i;
133+
}
134+
111135

112136

113137
bool
@@ -174,6 +198,17 @@ BmpInput::open(const std::string& name, ImageSpec& newspec,
174198
return false;
175199
}
176200

201+
// Compute channel shifts & masks (only relevant for 16bpp case)
202+
if (m_dib_header.red_mask == 0 || m_dib_header.green_mask == 0
203+
|| m_dib_header.blue_mask == 0) {
204+
m_dib_header.red_mask = 0b111110000000000;
205+
m_dib_header.green_mask = 0b000001111100000;
206+
m_dib_header.blue_mask = 0b000000000011111;
207+
}
208+
calc_shifts(m_dib_header.red_mask, m_bit_counts[0], m_right_shifts[0]);
209+
calc_shifts(m_dib_header.green_mask, m_bit_counts[1], m_right_shifts[1]);
210+
calc_shifts(m_dib_header.blue_mask, m_bit_counts[2], m_right_shifts[2]);
211+
177212
// computing size of one scanline - this is the size of one scanline that
178213
// is stored in the file, not in the memory
179214
int swidth = 0;
@@ -184,7 +219,7 @@ BmpInput::open(const std::string& name, ImageSpec& newspec,
184219
break;
185220
case 16:
186221
m_padded_scanline_size = ((m_spec.width << 1) + 3) & ~3;
187-
m_spec.attribute("oiio:BitsPerSample", 4);
222+
m_spec.attribute("oiio:BitsPerSample", m_bit_counts[0]);
188223
break;
189224
case 8:
190225
m_padded_scanline_size = (m_spec.width + 3) & ~3;
@@ -354,14 +389,20 @@ BmpInput::read_native_scanline(int subimage, int miplevel, int y, int /*z*/,
354389
}
355390

356391
if (m_dib_header.bpp == 16) {
357-
const uint16_t RED = 0x7C00;
358-
const uint16_t GREEN = 0x03E0;
359-
const uint16_t BLUE = 0x001F;
360392
for (unsigned int i = 0, j = 0; j < scanline_bytes; i += 2, j += 3) {
361-
uint16_t pixel = (uint16_t) * (&fscanline[i]);
362-
mscanline[j] = (uint8_t)((pixel & RED) >> 8);
363-
mscanline[j + 1] = (uint8_t)((pixel & GREEN) >> 4);
364-
mscanline[j + 2] = (uint8_t)(pixel & BLUE);
393+
uint16_t pixel = *(uint16_t*)&fscanline[i];
394+
mscanline[j + 0]
395+
= (uint8_t)bit_range_convert((pixel & m_dib_header.red_mask)
396+
>> m_right_shifts[0],
397+
m_bit_counts[0], 8);
398+
mscanline[j + 1]
399+
= (uint8_t)bit_range_convert((pixel & m_dib_header.green_mask)
400+
>> m_right_shifts[1],
401+
m_bit_counts[1], 8);
402+
mscanline[j + 2]
403+
= (uint8_t)bit_range_convert((pixel & m_dib_header.blue_mask)
404+
>> m_right_shifts[2],
405+
m_bit_counts[2], 8);
365406
}
366407
}
367408
if (m_dib_header.bpp == 8) {

‎testsuite/bmp/ref/out.txt

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -206,12 +206,32 @@ Reading ../oiio-images/bmpsuite/g32def.bmp
206206
bmp:version: 3
207207
Comparing "../oiio-images/bmpsuite/g32def.bmp" and "g32def.bmp"
208208
PASS
209-
Reading ../oiio-images/bmpsuite/g32bf.bmp
210-
../oiio-images/bmpsuite/g32bf.bmp : 127 x 64, 4 channel, uint8 bmp
211-
SHA-1: D8807C680B17C70CB33B43AC072E0A9121C551B4
212-
channel list: R, G, B, A
209+
Reading ../oiio-images/bmpsuite/g16bf555.bmp
210+
../oiio-images/bmpsuite/g16bf555.bmp : 127 x 64, 3 channel, uint5 bmp
211+
SHA-1: F040412A98C1F9B55F377BA74418D79FEA149DD5
212+
channel list: R, G, B
213+
bmp:bitsperpixel: 16
213214
bmp:version: 3
214-
Comparing "../oiio-images/bmpsuite/g32bf.bmp" and "g32bf.bmp"
215+
oiio:BitsPerSample: 5
216+
Comparing "../oiio-images/bmpsuite/g16bf555.bmp" and "g16bf555.bmp"
217+
PASS
218+
Reading ../oiio-images/bmpsuite/g16bf565.bmp
219+
../oiio-images/bmpsuite/g16bf565.bmp : 127 x 64, 3 channel, uint5 bmp
220+
SHA-1: 61E14F82A64F3E6A57A7CABC61FD47E52E031B16
221+
channel list: R, G, B
222+
bmp:bitsperpixel: 16
223+
bmp:version: 3
224+
oiio:BitsPerSample: 5
225+
Comparing "../oiio-images/bmpsuite/g16bf565.bmp" and "g16bf565.bmp"
226+
PASS
227+
Reading ../oiio-images/bmpsuite/g16def555.bmp
228+
../oiio-images/bmpsuite/g16def555.bmp : 127 x 64, 3 channel, uint5 bmp
229+
SHA-1: F040412A98C1F9B55F377BA74418D79FEA149DD5
230+
channel list: R, G, B
231+
bmp:bitsperpixel: 16
232+
bmp:version: 3
233+
oiio:BitsPerSample: 5
234+
Comparing "../oiio-images/bmpsuite/g16def555.bmp" and "g16def555.bmp"
215235
PASS
216236
Reading src/g01bg2-v5.bmp
217237
src/g01bg2-v5.bmp : 127 x 64, 3 channel, uint8 bmp

‎testsuite/bmp/run.py

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,11 @@
1111
"g08res11.bmp", "g08res21.bmp", "g08res22.bmp",
1212
"g08s0.bmp", "g08w124.bmp", "g08w125.bmp", "g08w126.bmp",
1313
"g08rle.bmp", "g08offs.bmp",
14-
"g24.bmp", "g32bf.bmp", "g32def.bmp", "g32bf.bmp"
15-
]
14+
"g24.bmp", "g32bf.bmp", "g32def.bmp",
15+
"g16bf555.bmp", "g16bf565.bmp", "g16def555.bmp" ]
1616
for f in files :
1717
command += rw_command (OIIO_TESTSUITE_IMAGEDIR, f)
1818

19-
# TODO: seems broken: g16bf555.bmp,
20-
# g16bf565.bmp, g16def555.bmp
21-
2219
# Test BMR version 5
2320
command += rw_command ("src", "g01bg2-v5.bmp")
2421

0 commit comments

Comments
 (0)