Skip to content

Support reading pixel values from color 16 bit images #699

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Aug 19, 2025
Merged

Support reading pixel values from color 16 bit images #699

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5d8d665
Handle color 16 bit images
iche033 Aug 15, 2025
e9b5fdc
add test files
iche033 Aug 15, 2025
0e307e7
refactor
iche033 Aug 18, 2025
b71e351
lint and build fix
iche033 Aug 18, 2025
3276a52
cleanup
iche033 Aug 18, 2025
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
2 changes: 2 additions & 0 deletions graphics/include/gz/common/Image.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ namespace gz
"RGBA_INT8",
"BGRA_INT8",
"RGB_INT16",
"RGBA_INT16",
"RGB_INT32",
"BGR_INT8",
"BGR_INT16",
Expand Down Expand Up @@ -72,6 +73,7 @@ namespace gz
RGBA_INT8,
BGRA_INT8,
RGB_INT16,
RGBA_INT16,
RGB_INT32,
BGR_INT8,
BGR_INT16,
Expand Down
158 changes: 84 additions & 74 deletions graphics/src/Image.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#endif
#include <FreeImage.h>

#include <cstdint>
#include <cstring>
#include <string>

Expand All @@ -37,11 +38,17 @@ namespace gz
class Image::Implementation
{
/// \brief bitmap data
public: FIBITMAP *bitmap;
public: FIBITMAP *bitmap{nullptr};

/// \brief path name of the image file
public: std::string fullName;

/// \brief Color type for this image
public: FREE_IMAGE_COLOR_TYPE colorType{FIC_RGB};

/// \brief Image type, i.e. pixel format
public: FREE_IMAGE_TYPE imageType{FIT_UNKNOWN};

/// \brief Implementation of Data, returns vector of bytes
public: std::vector<unsigned char> DataImpl(FIBITMAP *_img) const;

Expand Down Expand Up @@ -150,6 +157,9 @@ int Image::Load(const std::string &_filename)
return -1;
}

this->dataPtr->colorType = FreeImage_GetColorType(this->dataPtr->bitmap);
this->dataPtr->imageType = FreeImage_GetImageType(this->dataPtr->bitmap);

return 0;
}

Expand Down Expand Up @@ -257,6 +267,8 @@ void Image::SetFromData(const unsigned char *_data,
this->Height());
FreeImage_Unload(toDelete);
}
this->dataPtr->colorType = FreeImage_GetColorType(this->dataPtr->bitmap);
this->dataPtr->imageType = FreeImage_GetImageType(this->dataPtr->bitmap);
}

//////////////////////////////////////////////////
Expand Down Expand Up @@ -285,6 +297,8 @@ void Image::SetFromCompressedData(unsigned char *_data,
FIMEMORY *fiMem = FreeImage_OpenMemory(_data, _size);
this->dataPtr->bitmap = FreeImage_LoadFromMemory(format, fiMem);
FreeImage_CloseMemory(fiMem);
this->dataPtr->colorType = FreeImage_GetColorType(this->dataPtr->bitmap);
this->dataPtr->imageType = FreeImage_GetImageType(this->dataPtr->bitmap);
}
else
{
Expand Down Expand Up @@ -415,16 +429,22 @@ math::Color Image::Pixel(unsigned int _x, unsigned int _y) const
if (!this->Valid())
return clr;

FREE_IMAGE_COLOR_TYPE type = FreeImage_GetColorType(this->dataPtr->bitmap);
if (_x >= this->Width() || _y >= this->Height())
{
gzerr << "Image: Coordinates out of range["
<< _x << ", " << _y << "] \n";
return clr;
}

if (type == FIC_RGB || type == FIC_RGBALPHA)
if ((this->dataPtr->colorType == FIC_RGB ||
this->dataPtr->colorType == FIC_RGBALPHA) &&
(this->dataPtr->imageType == FIT_BITMAP))
{
RGBQUAD firgb;

if (FreeImage_GetPixelColor(this->dataPtr->bitmap, _x, _y, &firgb) == FALSE)
{
gzerr << "Image: Coordinates out of range["
<< _x << " " << _y << "] \n";
gzerr << "Failed to get pixel value at ["
<< _x << ", " << _y << "] \n";
return clr;
}
clr.Set(firgb.rgbRed / 255.0f, firgb.rgbGreen / 255.0f,
Expand All @@ -435,8 +455,8 @@ math::Color Image::Pixel(unsigned int _x, unsigned int _y) const
if (this->dataPtr->PixelIndex(
this->dataPtr->bitmap, _x, _y, clr) == FALSE)
{
gzerr << "Image: Coordinates out of range ["
<< _x << " " << _y << "] \n";
gzerr << "Failed to get pixel value at ["
<< _x << ", " << _y << "] \n";
return clr;
}
}
Expand Down Expand Up @@ -473,64 +493,20 @@ math::Color Image::AvgColor() const
//////////////////////////////////////////////////
math::Color Image::MaxColor() const
{
unsigned int x, y;
math::Color clr;
math::Color maxClr;

maxClr.Set(0, 0, 0, 0);

if (!this->Valid())
return clr;

FREE_IMAGE_COLOR_TYPE type = FreeImage_GetColorType(this->dataPtr->bitmap);

if (type == FIC_RGB || type == FIC_RGBALPHA)
{
RGBQUAD firgb;
return maxClr;

for (y = 0; y < this->Height(); y++)
{
for (x = 0; x < this->Width(); x++)
{
clr.Set(0, 0, 0, 0);

if (FALSE ==
FreeImage_GetPixelColor(this->dataPtr->bitmap, x, y, &firgb))
{
gzerr << "Image: Coordinates out of range["
<< x << " " << y << "] \n";
continue;
}
clr.Set(firgb.rgbRed / 255.0f, firgb.rgbGreen / 255.0f,
firgb.rgbBlue / 255.0f);

if (clr.R() + clr.G() + clr.B() > maxClr.R() + maxClr.G() + maxClr.B())
{
maxClr = clr;
}
}
}
}
else
maxClr.Set(0, 0, 0, 0);
for (unsigned int y = 0; y < this->Height(); y++)
{
for (y = 0; y < this->Height(); y++)
for (unsigned int x = 0; x < this->Width(); x++)
{
for (x = 0; x < this->Width(); x++)
math::Color clr = this->Pixel(x, y);
if (clr.R() + clr.G() + clr.B() > maxClr.R() + maxClr.G() + maxClr.B())
{
clr.Set(0, 0, 0, 0);

if (this->dataPtr->PixelIndex(
this->dataPtr->bitmap, x, y, clr) == FALSE)
{
gzerr << "Image: Coordinates out of range ["
<< x << " " << y << "] \n";
continue;
}

if (clr.R() + clr.G() + clr.B() > maxClr.R() + maxClr.G() + maxClr.B())
{
maxClr = clr;
}
maxClr = clr;
}
}
}
Expand All @@ -545,9 +521,11 @@ BOOL Image::Implementation::PixelIndex(
if (!_dib)
return FALSE;

FREE_IMAGE_TYPE imageType = FreeImage_GetImageType(_dib);
if (_x >= FreeImage_GetWidth(_dib) || _y >= FreeImage_GetHeight(_dib))
return FALSE;

// 8 bit images
if (imageType == FIT_BITMAP)
if (this->imageType == FIT_BITMAP)
{
BYTE byteValue;
// FreeImage_GetPixelIndex should also work with 1 and 4 bit images
Expand All @@ -563,21 +541,51 @@ BOOL Image::Implementation::PixelIndex(
_color.Set(value, value, value);
}
// 16 bit images
else if (imageType == FIT_UINT16)
else if (this->imageType == FIT_UINT16)
{
if ((_x < FreeImage_GetWidth(_dib)) && (_y < FreeImage_GetHeight(_dib)))
{
WORD *bits = reinterpret_cast<WORD *>(FreeImage_GetScanLine(_dib, _y));
uint16_t word = static_cast<uint16_t>(bits[_x]);
// convert to float value between 0-1
float value = word / static_cast<float>(math::MAX_UI16);
_color.Set(value, value, value);
}
else
{
return FALSE;
}
WORD *bits = reinterpret_cast<WORD *>(FreeImage_GetScanLine(_dib, _y));
uint16_t word = static_cast<uint16_t>(bits[_x]);
// convert to float value between 0-1
float value = word / static_cast<float>(math::MAX_UI16);
_color.Set(value, value, value);
}
else if (this->imageType == FIT_INT16)
{
WORD *bits = reinterpret_cast<WORD *>(FreeImage_GetScanLine(_dib, _y));
int16_t word = static_cast<int16_t>(bits[_x]);
// convert to float value between 0-1
float value = word / static_cast<float>(math::MAX_I16);
_color.Set(value, value, value);
}
else if (this->imageType == FIT_RGB16)
{
const unsigned int channels = 3u;
WORD *bits = reinterpret_cast<WORD *>(FreeImage_GetScanLine(_dib, _y));
uint16_t r = static_cast<uint16_t>(bits[_x * channels]);
uint16_t g = static_cast<uint16_t>(bits[_x * channels + 1]);
uint16_t b = static_cast<uint16_t>(bits[_x * channels + 2]);
// convert to float value between 0-1
float valueR = r / static_cast<float>(math::MAX_UI16);
float valueG = g / static_cast<float>(math::MAX_UI16);
float valueB = b / static_cast<float>(math::MAX_UI16);
_color.Set(valueR, valueG, valueB);
}
else if (this->imageType == FIT_RGBA16)
{
const unsigned int channels = 4u;
WORD *bits = reinterpret_cast<WORD *>(FreeImage_GetScanLine(_dib, _y));
uint16_t r = static_cast<uint16_t>(bits[_x * channels]);
uint16_t g = static_cast<uint16_t>(bits[_x * channels + 1]);
uint16_t b = static_cast<uint16_t>(bits[_x * channels + 2]);
uint16_t a = static_cast<uint16_t>(bits[_x * channels + 3]);
// convert to float value between 0-1
float valueR = r / static_cast<float>(math::MAX_UI16);
float valueG = g / static_cast<float>(math::MAX_UI16);
float valueB = b / static_cast<float>(math::MAX_UI16);
float valueA = a / static_cast<float>(math::MAX_UI16);
_color.Set(valueR, valueG, valueB, valueA);
}

return TRUE;
}

Expand Down Expand Up @@ -634,6 +642,8 @@ Image::PixelFormatType Image::PixelFormat() const
}
else if (type == FIT_RGB16)
fmt = RGB_INT16;
else if (type == FIT_RGBA16)
fmt = RGBA_INT16;
else if (type == FIT_RGBF)
fmt = RGB_FLOAT32;
else if (type == FIT_UINT16 || type == FIT_INT16)
Expand Down
59 changes: 59 additions & 0 deletions graphics/src/Image_TEST.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*
*/
#include <fstream>
#include <string>

#include <gtest/gtest.h>

Expand Down Expand Up @@ -413,6 +414,8 @@ TEST_F(ImageTest, ConvertPixelFormat)
Image::ConvertPixelFormat("RGBA_INT8"));
EXPECT_EQ(Image::PixelFormatType::RGB_INT16,
Image::ConvertPixelFormat("RGB_INT16"));
EXPECT_EQ(Image::PixelFormatType::RGBA_INT16,
Image::ConvertPixelFormat("RGBA_INT16"));
EXPECT_EQ(Image::PixelFormatType::RGB_INT32,
Image::ConvertPixelFormat("RGB_INT32"));
EXPECT_EQ(Image::PixelFormatType::BGR_INT8,
Expand Down Expand Up @@ -673,6 +676,62 @@ TEST_F(ImageTest, Grayscale)
}
}


/////////////////////////////////////////////////
TEST_F(ImageTest, Color16bit)
Copy link
Member

Choose a reason for hiding this comment

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

I noticed this test doesn't actually test the API that had the problematic behavior (Image.Pixel(x, y)) or rather, it only does so in a indirect way, specifically it only tests img.MaxColor(), which in this specific code path calls Image::Implementation::PixelIndex which happens to be what also Pixel would call because their logic is duplicated.

I have two alternative proposals:

  1. (My preference), I saw that MaxColor and Pixel have pretty much the same duplicated logic, which also shows in this PR's diff that has the same change implemented twice. What if we refactored so all MaxColor does is effectively just call Pixel in a loop and we only had the logic in Pixel?
  2. If the above is not possible, we should at least add a test for Pixel in these cases, so we make sure that function's implementation is correct as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep good idea, went with 1 and done in 0e307e7. I noticed that after refactoring MaxColor() to call Pixel(...), the Image test took a long time to run. Turns out it's due to repeatedly calling FreeImage_GetColorType which seems to be an expensive call. I now made it a member variable and initialize it when the image is first loaded.

{
{
common::Image img;
std::string fileName = common::testing::TestFile("data",
"rgb_16bit.png");
EXPECT_EQ(0, img.Load(fileName));
const unsigned int width = 4u;
const unsigned int height = 4u;
const unsigned int channels = 3u;
const unsigned int bpp = channels * 16u;
EXPECT_TRUE(img.Valid());
EXPECT_EQ(width, img.Width());
EXPECT_EQ(height, img.Height());
EXPECT_EQ(bpp, img.BPP());
EXPECT_EQ(width * bpp / 8u, img.Pitch());
EXPECT_EQ(common::Image::PixelFormatType::RGB_INT16, img.PixelFormat());
math::Color maxColor(0.0f, 0.0f, 0.847f);
EXPECT_NEAR(maxColor.R(), img.MaxColor().R(), 1e-3);
EXPECT_NEAR(maxColor.G(), img.MaxColor().G(), 1e-3);
EXPECT_NEAR(maxColor.B(), img.MaxColor().B(), 1e-3);
math::Color pixelWithMaxColor = img.Pixel(3, 1);
EXPECT_NEAR(maxColor.R(), pixelWithMaxColor.R(), 1e-3);
EXPECT_NEAR(maxColor.G(), pixelWithMaxColor.G(), 1e-3);
EXPECT_NEAR(maxColor.B(), pixelWithMaxColor.B(), 1e-3);
}
{
common::Image img;
std::string fileName = common::testing::TestFile("data",
"rgba_16bit.png");
EXPECT_EQ(0, img.Load(fileName));
const unsigned int width = 4u;
const unsigned int height = 4u;
const unsigned int channels = 4u;
const unsigned int bpp = channels * 16u;
EXPECT_TRUE(img.Valid());
EXPECT_EQ(width, img.Width());
EXPECT_EQ(height, img.Height());
EXPECT_EQ(bpp, img.BPP());
EXPECT_EQ(width * bpp / 8u, img.Pitch());
EXPECT_EQ(common::Image::PixelFormatType::RGBA_INT16, img.PixelFormat());
math::Color maxColor(0.0f, 0.0f, 0.847f, 0.5f);
EXPECT_NEAR(maxColor.R(), img.MaxColor().R(), 1e-3);
EXPECT_NEAR(maxColor.G(), img.MaxColor().G(), 1e-3);
EXPECT_NEAR(maxColor.B(), img.MaxColor().B(), 1e-3);
EXPECT_NEAR(maxColor.A(), img.MaxColor().A(), 1e-3);
math::Color pixelWithMaxColor = img.Pixel(3, 1);
EXPECT_NEAR(maxColor.R(), pixelWithMaxColor.R(), 1e-3);
EXPECT_NEAR(maxColor.G(), pixelWithMaxColor.G(), 1e-3);
EXPECT_NEAR(maxColor.B(), pixelWithMaxColor.B(), 1e-3);
EXPECT_NEAR(maxColor.A(), pixelWithMaxColor.A(), 1e-3);
}
}

using string_int2 = std::tuple<const char *, unsigned int, unsigned int>;

class ImagePerformanceTest : public ImageTest,
Expand Down
Binary file added test/data/rgb_16bit.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added test/data/rgba_16bit.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading