Skip to content

Commit

Permalink
Respect sRGB color space in conversion to grayscale image formats.
Browse files Browse the repository at this point in the history 
This behavior is also used for setPixel() when choosing to do grayscale
conversion.

Use "no grayscale" implementation for set pixel when no grayscale
conversion can possibly occur for internal functions (such as resize) to
eliminate unnecessary work.
  • Loading branch information
@akb825
akb825 committed Nov 11, 2024
1 parent fded43f commit 024bc57
Show file tree
Hide file tree
Showing 2 changed files with 120 additions and 23 deletions.
77 changes: 55 additions & 22 deletions lib/src/Image.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1101,7 +1101,17 @@ bool Image::setPixel(unsigned int x, unsigned int y, const ColorRGBAd& color, bo

void* scanline = getScanlineImpl(m_impl->image, m_impl->height, y);
if (convertGrayscale)
{
if (m_impl->colorSpace == ColorSpace::sRGB && isGrayscaleFormat(m_impl->format))
{
ColorRGBAd grayColor;
grayColor.r = grayColor.g = grayColor.b = linearToSRGB(toGrayscale(
sRGBToLinear(color.r), sRGBToLinear(color.g), sRGBToLinear(color.b)));
grayColor.a = color.a;
setPixelNoGrayscaleImpl(m_impl->format, scanline, x, grayColor);
}
return setPixelImpl(m_impl->format, scanline, x, color);
}
return setPixelNoGrayscaleImpl(m_impl->format, scanline, x, color);
}

Expand All @@ -1123,21 +1133,25 @@ Image Image::convert(Format dstFormat, bool convertGrayscale) const
if (type == FIT_UNKNOWN)
return image;

// Only allow FreeImage grayscale conversion for linear color space.
convertGrayscale = convertGrayscale && isGrayscaleFormat(dstFormat);
bool convertLinearGrayscale = convertGrayscale && m_impl->colorSpace == ColorSpace::Linear;

// Special case: UInt16 type gets treated by FreeImage as Gray16, so always use fallback
// for UInt16.
if (srcFormat != Format::UInt16)
{
switch (dstFormat)
{
case Format::Gray8:
if (convertGrayscale || isGrayscaleFormat(srcFormat))
if (convertLinearGrayscale || isGrayscaleFormat(srcFormat))
{
image.m_impl.reset(Impl::create(
FreeImage_ConvertTo8Bits(m_impl->image), m_impl->colorSpace, dstFormat));
}
break;
case Format::Gray16:
if (convertGrayscale || isGrayscaleFormat(srcFormat))
if (convertLinearGrayscale || isGrayscaleFormat(srcFormat))
{
image.m_impl.reset(Impl::create(
FreeImage_ConvertToType(m_impl->image, FIT_UINT16), m_impl->colorSpace,
Expand Down Expand Up @@ -1205,7 +1219,7 @@ Image Image::convert(Format dstFormat, bool convertGrayscale) const
case Format::Float:
// NOTE: Don't use FreeImage conversion for float to float conversion to avoid
// clamping HDR images. Also need to use fallback if not converting to grayscale.
if ((convertGrayscale || isGrayscaleFormat(srcFormat)) &&
if ((convertLinearGrayscale || isGrayscaleFormat(srcFormat)) &&
!needFloatConvertFallback(srcFormat))
{
image.m_impl.reset(Impl::create(
Expand All @@ -1216,7 +1230,7 @@ Image Image::convert(Format dstFormat, bool convertGrayscale) const
case Format::Double:
// NOTE: Don't use FreeImage conversion for float to float conversion to avoid
// clamping HDR images. Also need to use fallback if not converting to grayscale.
if ((convertGrayscale || isGrayscaleFormat(srcFormat)) &&
if ((convertLinearGrayscale || isGrayscaleFormat(srcFormat)) &&
!needFloatConvertFallback(srcFormat))
{
image.m_impl.reset(Impl::create(
Expand Down Expand Up @@ -1244,14 +1258,33 @@ Image Image::convert(Format dstFormat, bool convertGrayscale) const
// Never convert grayscale when going from complex type.
if (convertGrayscale && srcFormat != Format::Complex)
{
for (unsigned int y = 0; y < m_impl->height; ++y)
if (m_impl->colorSpace == ColorSpace::Linear)
{
const void* srcScanline = FreeImage_GetScanLine(m_impl->image, y);
void* dstScanline = FreeImage_GetScanLine(image.m_impl->image, y);
for (unsigned int x = 0; x < m_impl->width; ++x)
for (unsigned int y = 0; y < m_impl->height; ++y)
{
getPixelImpl(color, m_impl->format, srcScanline, x);
setPixelImpl(image.m_impl->format, dstScanline, x, color);
const void* srcScanline = FreeImage_GetScanLine(m_impl->image, y);
void* dstScanline = FreeImage_GetScanLine(image.m_impl->image, y);
for (unsigned int x = 0; x < m_impl->width; ++x)
{
getPixelImpl(color, m_impl->format, srcScanline, x);
setPixelImpl(image.m_impl->format, dstScanline, x, color);
}
}
}
else
{
// Perform grayscale conversion in linear space.
for (unsigned int y = 0; y < m_impl->height; ++y)
{
const void* srcScanline = FreeImage_GetScanLine(m_impl->image, y);
void* dstScanline = FreeImage_GetScanLine(image.m_impl->image, y);
for (unsigned int x = 0; x < m_impl->width; ++x)
{
getPixelImpl(color, m_impl->format, srcScanline, x);
color.r = color.g = color.b = linearToSRGB(toGrayscale(
sRGBToLinear(color.r), sRGBToLinear(color.g), sRGBToLinear(color.b)));
setPixelNoGrayscaleImpl(image.m_impl->format, dstScanline, x, color);
}
}
}
}
Expand Down Expand Up @@ -1393,7 +1426,7 @@ Image Image::resize(unsigned int width, unsigned int height, ResizeFilter filter
color.g /= totalScale;
color.b /= totalScale;
color.a /= totalScale;
setPixelImpl(image.m_impl->format, dstScanline, x, color);
setPixelNoGrayscaleImpl(image.m_impl->format, dstScanline, x, color);
}
}
break;
Expand Down Expand Up @@ -1451,7 +1484,7 @@ Image Image::resize(unsigned int width, unsigned int height, ResizeFilter filter
color.g /= totalScale;
color.b /= totalScale;
color.a /= totalScale;
setPixelImpl(image.m_impl->format, dstScanline, x, color);
setPixelNoGrayscaleImpl(image.m_impl->format, dstScanline, x, color);
}
}
break;
Expand Down Expand Up @@ -1506,8 +1539,8 @@ Image Image::rotate(RotateAngle angle) const
{
getPixelImpl(color, m_impl->format, srcScanline, x);
void* dstScanline = FreeImage_GetScanLine(image.m_impl->image, x);
setPixelImpl(image.m_impl->format, dstScanline, image.m_impl->width - y - 1,
color);
setPixelNoGrayscaleImpl(image.m_impl->format, dstScanline,
image.m_impl->width - y - 1, color);
}
}
break;
Expand All @@ -1525,8 +1558,8 @@ Image Image::rotate(RotateAngle angle) const
for (unsigned int x = 0; x < m_impl->width; ++x)
{
getPixelImpl(color, m_impl->format, srcScanline, x);
setPixelImpl(image.m_impl->format, dstScanline, m_impl->width - x - 1,
color);
setPixelNoGrayscaleImpl(image.m_impl->format, dstScanline,
m_impl->width - x - 1, color);
}
}
break;
Expand All @@ -1544,7 +1577,7 @@ Image Image::rotate(RotateAngle angle) const
getPixelImpl(color, m_impl->format, srcScanline,
m_impl->width - x - 1);
void* dstScanline = FreeImage_GetScanLine(image.m_impl->image, x);
setPixelImpl(image.m_impl->format, dstScanline, y, color);
setPixelNoGrayscaleImpl(image.m_impl->format, dstScanline, y, color);
}
}
break;
Expand Down Expand Up @@ -1607,7 +1640,7 @@ bool Image::preMultiplyAlpha()
color.b = linearToSRGB(color.b);
}

setPixelImpl(m_impl->format, scanline, x, color);
setPixelNoGrayscaleImpl(m_impl->format, scanline, x, color);
}
}
default:
Expand Down Expand Up @@ -1637,7 +1670,7 @@ bool Image::changeColorSpace(ColorSpace colorSpace)
color.r = sRGBToLinear(color.r);
color.g = sRGBToLinear(color.g);
color.b = sRGBToLinear(color.b);
setPixelImpl(m_impl->format, scanline, x, color);
setPixelNoGrayscaleImpl(m_impl->format, scanline, x, color);
}
}
}
Expand All @@ -1654,7 +1687,7 @@ bool Image::changeColorSpace(ColorSpace colorSpace)
color.r = linearToSRGB(color.r);
color.g = linearToSRGB(color.g);
color.b = linearToSRGB(color.b);
setPixelImpl(m_impl->format, scanline, x, color);
setPixelNoGrayscaleImpl(m_impl->format, scanline, x, color);
}
}
}
Expand Down Expand Up @@ -1690,7 +1723,7 @@ bool Image::grayscale()
grayscale = linearToSRGB(grayscale);
color.r = color.g = color.b = grayscale;

setPixelImpl(m_impl->format, scanline, x, color);
setPixelNoGrayscaleImpl(m_impl->format, scanline, x, color);
}
}

Expand Down Expand Up @@ -1725,7 +1758,7 @@ bool Image::swizzle(Channel red, Channel green, Channel blue, Channel alpha)
swzl.a = reinterpret_cast<double*>(&color)[static_cast<unsigned int>(alpha)];
else
swzl.a = 1;
setPixelImpl(m_impl->format, scanline, x, swzl);
setPixelNoGrayscaleImpl(m_impl->format, scanline, x, swzl);
}
}

Expand Down
66 changes: 65 additions & 1 deletion lib/test/ImageTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,11 @@ TEST(ImageConvertTest, GrayscaleConversions)
EXPECT_TRUE(otherImage.isValid());
EXPECT_TRUE(otherImage.getPixel(convertedColor, 0, 0));
EXPECT_NEAR(color.r, convertedColor.r, epsilon);

// Calling setPixel() should have the same behavior as convert().
otherImage.setPixel(0, 0, color);
EXPECT_TRUE(otherImage.getPixel(convertedColor, 0, 0));
EXPECT_NEAR(grayscale, convertedColor.r, epsilon);
}
}

Expand Down Expand Up @@ -465,6 +470,65 @@ TEST(ImageConvertTest, GrayscaleConversions)
EXPECT_EQ(0, convertedColor.r);
}

TEST(ImageConvertTest, sRGBGrayscaleConversions)
{
const double epsilon = 2.2e-2;

ColorRGBAd color;
color.r = 0.25;
color.g = 0.5;
color.b = 0.75;
color.a = 1.0;
double grayscale = linearToSRGB(toGrayscale(
sRGBToLinear(color.r), sRGBToLinear(color.g), sRGBToLinear(color.b)));

std::vector<Image::Format> srcFormats =
{
Image::Format::RGB5,
Image::Format::RGB565,
Image::Format::RGB8,
Image::Format::RGB16,
Image::Format::RGBF,
Image::Format::RGBA8,
Image::Format::RGBA16,
Image::Format::RGBAF
};

std::vector<Image::Format> dstFormats =
{
Image::Format::Gray8,
Image::Format::Gray16,
Image::Format::Float,
Image::Format::Double
};

for (Image::Format srcFormat : srcFormats)
{
Image image;
EXPECT_TRUE(image.initialize(srcFormat, 1, 1, ColorSpace::sRGB));
EXPECT_TRUE(image.setPixel(0, 0, color));

for (Image::Format dstFormat : dstFormats)
{
Image otherImage = image.convert(dstFormat);
EXPECT_TRUE(otherImage.isValid());
ColorRGBAd convertedColor;
EXPECT_TRUE(otherImage.getPixel(convertedColor, 0, 0));
EXPECT_NEAR(grayscale, convertedColor.r, epsilon);

otherImage = image.convert(dstFormat, false);
EXPECT_TRUE(otherImage.isValid());
EXPECT_TRUE(otherImage.getPixel(convertedColor, 0, 0));
EXPECT_NEAR(color.r, convertedColor.r, epsilon);

// Calling setPixel() should have the same behavior as convert().
otherImage.setPixel(0, 0, color);
EXPECT_TRUE(otherImage.getPixel(convertedColor, 0, 0));
EXPECT_NEAR(grayscale, convertedColor.r, epsilon);
}
}
}

TEST(ImageConvertTest, UInt16Conversions)
{
// FreeImage native handling of UInt16 type is same as Gray16 rather than like other
Expand Down Expand Up @@ -1264,13 +1328,13 @@ INSTANTIATE_TEST_SUITE_P(ImageTestTypes,
ImageTest,
testing::Values(
ImageTestInfo(Image::Format::Gray8, 1/255.0, 0),
ImageTestInfo(Image::Format::Gray16, 1/255.0, 0),
ImageTestInfo(Image::Format::RGB5, 1/31.0, 3),
ImageTestInfo(Image::Format::RGB565, 1/31.0, 3),
ImageTestInfo(Image::Format::RGB8, 1/255.0, 3),
ImageTestInfo(Image::Format::RGB16, 1/65535.0, 3),
ImageTestInfo(Image::Format::RGBF, 1e-6, 3),
ImageTestInfo(Image::Format::RGBA8, 1/255.0, 4),
ImageTestInfo(Image::Format::Gray16, 1/255.0, 0),
ImageTestInfo(Image::Format::RGBA16, 1/65535.0, 4),
ImageTestInfo(Image::Format::RGBAF, 1e-6, 4),
ImageTestInfo(Image::Format::Int16, 1, 1),
Expand Down

0 comments on commit 024bc57

Please sign in to comment.