diff --git a/src/ico.imageio/icooutput.cpp b/src/ico.imageio/icooutput.cpp index 27505395fb..18e5fdb5a3 100644 --- a/src/ico.imageio/icooutput.cpp +++ b/src/ico.imageio/icooutput.cpp @@ -319,12 +319,13 @@ ICOOutput::open(const std::string& name, const ImageSpec& userspec, // unused still, should do conversion to unassociated bool convert_alpha; float gamma; + bool srgb; png_init_io(m_png, m_file); png_set_compression_level(m_png, Z_BEST_COMPRESSION); PNG_pvt::write_info(m_png, m_info, m_color_type, m_spec, m_pngtext, - convert_alpha, gamma); + convert_alpha, srgb, gamma); } else { // write DIB header ico_bitmapinfo bmi; diff --git a/src/png.imageio/png_pvt.h b/src/png.imageio/png_pvt.h index ffdb24f8eb..b0be93c0c9 100644 --- a/src/png.imageio/png_pvt.h +++ b/src/png.imageio/png_pvt.h @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -574,7 +575,8 @@ put_parameter(png_structp& sp, png_infop& ip, const std::string& _name, /// inline const std::string write_info(png_structp& sp, png_infop& ip, int& color_type, ImageSpec& spec, - std::vector& text, bool& convert_alpha, float& gamma) + std::vector& text, bool& convert_alpha, bool& srgb, + float& gamma) { // Force either 16 or 8 bit integers if (spec.format == TypeDesc::UINT8 || spec.format == TypeDesc::INT8) @@ -598,11 +600,14 @@ write_info(png_structp& sp, png_infop& ip, int& color_type, ImageSpec& spec, gamma = spec.get_float_attribute("oiio:Gamma", 1.0); + const ColorConfig& colorconfig = ColorConfig::default_colorconfig(); string_view colorspace = spec.get_string_attribute("oiio:ColorSpace"); - if (Strutil::iequals(colorspace, "Linear")) { + if (colorconfig.equivalent(colorspace, "scene_linear") + || colorconfig.equivalent(colorspace, "linear")) { if (setjmp(png_jmpbuf(sp))) // NOLINT(cert-err52-cpp) return "Could not set PNG gAMA chunk"; png_set_gAMA(sp, ip, 1.0); + srgb = false; } else if (Strutil::istarts_with(colorspace, "Gamma")) { Strutil::parse_word(colorspace); float g = Strutil::from_string(colorspace); @@ -611,10 +616,12 @@ write_info(png_structp& sp, png_infop& ip, int& color_type, ImageSpec& spec, if (setjmp(png_jmpbuf(sp))) // NOLINT(cert-err52-cpp) return "Could not set PNG gAMA chunk"; png_set_gAMA(sp, ip, 1.0f / gamma); - } else if (Strutil::iequals(colorspace, "sRGB")) { + srgb = false; + } else if (colorconfig.equivalent(colorspace, "sRGB")) { if (setjmp(png_jmpbuf(sp))) // NOLINT(cert-err52-cpp) return "Could not set PNG gAMA and cHRM chunk"; png_set_sRGB_gAMA_and_cHRM(sp, ip, PNG_sRGB_INTENT_ABSOLUTE); + srgb = true; } // Write ICC profile, if we have anything diff --git a/src/png.imageio/pnginput.cpp b/src/png.imageio/pnginput.cpp index 2c58551826..5dfce4f140 100644 --- a/src/png.imageio/pnginput.cpp +++ b/src/png.imageio/pnginput.cpp @@ -44,9 +44,11 @@ class PNGInput final : public ImageInput { int m_subimage; ///< What subimage are we looking at? Imath::Color3f m_bg; ///< Background color int m_next_scanline; - bool m_keep_unassociated_alpha; ///< Do not convert unassociated alpha + bool m_keep_unassociated_alpha; ///< Do not convert unassociated alpha + bool m_srgb = false; ///< It's an sRGB image (not gamma) + bool m_err = false; + float m_gamma = 1.0f; std::unique_ptr m_config; // Saved copy of configuration spec - bool m_err = false; /// Reset everything to initial state /// @@ -58,7 +60,9 @@ class PNGInput final : public ImageInput { m_buf.clear(); m_next_scanline = 0; m_keep_unassociated_alpha = false; + m_srgb = false; m_err = false; + m_gamma = 1.0; m_config.reset(); ioproxy_clear(); } @@ -82,6 +86,10 @@ class PNGInput final : public ImageInput { png_chunk_error(png_ptr, pnginput->geterror(false).c_str()); } } + + template + static void associateAlpha(T* data, int size, int channels, + int alpha_channel, bool srgb, float gamma); }; @@ -159,6 +167,12 @@ PNGInput::open(const std::string& name, ImageSpec& newspec) return false; } + m_gamma = m_spec.get_float_attribute("oiio:Gamma", 1.0f); + string_view colorspace = m_spec.get_string_attribute("oiio:ColorSpace", + "sRGB"); + const ColorConfig& colorconfig(ColorConfig::default_colorconfig()); + m_srgb = colorconfig.equivalent(colorspace, "sRGB"); + newspec = spec(); m_next_scanline = 0; @@ -208,34 +222,45 @@ PNGInput::close() template -static void -png_associateAlpha(T* data, int size, int channels, int alpha_channel, - float gamma) +void +PNGInput::associateAlpha(T* data, int size, int channels, int alpha_channel, + bool srgb, float gamma) { - T max = std::numeric_limits::max(); - if (gamma == 1) { - for (int x = 0; x < size; ++x, data += channels) - for (int c = 0; c < channels; c++) - if (c != alpha_channel) { - unsigned int f = data[c]; - data[c] = (f * data[alpha_channel]) / max; + // We need to transform to linear space, associate the alpha, and then + // transform back. + if (srgb) { + for (int x = 0; x < size; ++x, data += channels) { + DataArrayProxy val(data); + float alpha = val[alpha_channel]; + if (alpha != 0.0f && alpha != 1.0f) { + for (int c = 0; c < channels; c++) { + if (c != alpha_channel) { + float f = sRGB_to_linear(val[c]); + val[c] = linear_to_sRGB(f * alpha); + } } - } else { //With gamma correction - float inv_max = 1.0 / max; + } + } + } else if (gamma == 1.0f) { for (int x = 0; x < size; ++x, data += channels) { - float alpha_associate = pow(data[alpha_channel] * inv_max, gamma); - // We need to transform to linear space, associate the alpha, and - // then transform back. That is, if D = data[c], we want - // - // D' = max * ( (D/max)^(1/gamma) * (alpha/max) ) ^ gamma - // - // This happens to simplify to something which looks like - // multiplying by a nonlinear alpha: - // - // D' = D * (alpha/max)^gamma - for (int c = 0; c < channels; c++) - if (c != alpha_channel) - data[c] = static_cast(data[c] * alpha_associate); + DataArrayProxy val(data); + float alpha = val[alpha_channel]; + if (alpha != 0.0f && alpha != 1.0f) { + for (int c = 0; c < channels; c++) + if (c != alpha_channel) + data[c] = data[c] * alpha; + } + } + } else { // With gamma correction + float inv_gamma = 1.0f / gamma; + for (int x = 0; x < size; ++x, data += channels) { + DataArrayProxy val(data); + float alpha = val[alpha_channel]; + if (alpha != 0.0f && alpha != 1.0f) { + for (int c = 0; c < channels; c++) + if (c != alpha_channel) + val[c] = powf((powf(val[c], gamma)) * alpha, inv_gamma); + } } } } @@ -295,13 +320,13 @@ PNGInput::read_native_scanline(int subimage, int miplevel, int y, int /*z*/, // PNG specifically dictates unassociated (un-"premultiplied") alpha. // Convert to associated unless we were requested not to do so. if (m_spec.alpha_channel != -1 && !m_keep_unassociated_alpha) { - float gamma = m_spec.get_float_attribute("oiio:Gamma", 1.0f); if (m_spec.format == TypeDesc::UINT16) - png_associateAlpha((unsigned short*)data, m_spec.width, - m_spec.nchannels, m_spec.alpha_channel, gamma); + associateAlpha((unsigned short*)data, m_spec.width, + m_spec.nchannels, m_spec.alpha_channel, m_srgb, + m_gamma); else - png_associateAlpha((unsigned char*)data, m_spec.width, - m_spec.nchannels, m_spec.alpha_channel, gamma); + associateAlpha((unsigned char*)data, m_spec.width, m_spec.nchannels, + m_spec.alpha_channel, m_srgb, m_gamma); } return true; diff --git a/src/png.imageio/pngoutput.cpp b/src/png.imageio/pngoutput.cpp index 4af31ee999..26e98362f9 100644 --- a/src/png.imageio/pngoutput.cpp +++ b/src/png.imageio/pngoutput.cpp @@ -44,10 +44,11 @@ class PNGOutput final : public ImageOutput { png_structp m_png; ///< PNG read structure pointer png_infop m_info; ///< PNG image info structure pointer unsigned int m_dither; - int m_color_type; ///< PNG color model type - bool m_convert_alpha; ///< Do we deassociate alpha? - bool m_need_swap; ///< Do we need to swap bytes? - float m_gamma; ///< Gamma to use for alpha conversion + int m_color_type; ///< PNG color model type + bool m_convert_alpha; ///< Do we deassociate alpha? + bool m_need_swap; ///< Do we need to swap bytes? + bool m_srgb = false; ///< It's an sRGB image (not gamma) + float m_gamma = 1.0f; ///< Gamma to use for alpha conversion std::vector m_scratch; std::vector m_pngtext; std::vector m_tilebuffer; @@ -60,10 +61,11 @@ class PNGOutput final : public ImageOutput { m_info = NULL; m_convert_alpha = true; m_need_swap = false; + m_srgb = false; + m_err = false; m_gamma = 1.0; m_pngtext.clear(); ioproxy_clear(); - m_err = false; } // Add a parameter to the output @@ -90,7 +92,7 @@ class PNGOutput final : public ImageOutput { template void deassociateAlpha(T* data, size_t npixels, int channels, - int alpha_channel, float gamma); + int alpha_channel, bool srgb, float gamma); }; @@ -204,7 +206,7 @@ PNGOutput::open(const std::string& name, const ImageSpec& userspec, #if defined(PNG_SKIP_sRGB_CHECK_PROFILE) && defined(PNG_SET_OPTION_SUPPORTED) // libpng by default checks ICC profiles and are very strict, treating - // it as a serious error if it doesn't match th profile it thinks is + // it as a serious error if it doesn't match the profile it thinks is // right for sRGB. This call disables that behavior, which tends to have // many false positives. Some references to discussion about this: // https://github.com/kornelski/pngquant/issues/190 @@ -214,7 +216,7 @@ PNGOutput::open(const std::string& name, const ImageSpec& userspec, #endif s = PNG_pvt::write_info(m_png, m_info, m_color_type, m_spec, m_pngtext, - m_convert_alpha, m_gamma); + m_convert_alpha, m_srgb, m_gamma); if (s.length()) { close(); @@ -273,9 +275,22 @@ PNGOutput::close() template void PNGOutput::deassociateAlpha(T* data, size_t npixels, int channels, - int alpha_channel, float gamma) + int alpha_channel, bool srgb, float gamma) { - if (gamma == 1) { + if (srgb) { + for (size_t x = 0; x < npixels; ++x, data += channels) { + DataArrayProxy val(data); + float alpha = val[alpha_channel]; + if (alpha != 0.0f && alpha != 1.0f) { + for (int c = 0; c < channels; c++) { + if (c != alpha_channel) { + float f = sRGB_to_linear(val[c]); + val[c] = linear_to_sRGB(f / alpha); + } + } + } + } + } else if (gamma == 1) { for (size_t x = 0; x < npixels; ++x, data += channels) { DataArrayProxy val(data); float alpha = val[alpha_channel]; @@ -331,7 +346,7 @@ PNGOutput::write_scanline(int y, int z, TypeDesc format, const void* data, TypeFloat, AutoStride, AutoStride, AutoStride); // Deassociate alpha deassociateAlpha(floatvals, size_t(m_spec.width), m_spec.nchannels, - m_spec.alpha_channel, m_gamma); + m_spec.alpha_channel, m_srgb, m_gamma); data = floatvals; format = TypeFloat; xstride = size_t(m_spec.nchannels) * sizeof(float); @@ -394,7 +409,7 @@ PNGOutput::write_scanlines(int ybegin, int yend, int z, TypeDesc format, AutoStride); // Deassociate alpha deassociateAlpha(floatvals, npixels, m_spec.nchannels, - m_spec.alpha_channel, m_gamma); + m_spec.alpha_channel, m_srgb, m_gamma); data = floatvals; format = TypeFloat; xstride = size_t(m_spec.nchannels) * sizeof(float); diff --git a/testsuite/png/ref/out-libpng15.txt b/testsuite/png/ref/out-libpng15.txt index ccf5dc428c..e8855cd272 100644 --- a/testsuite/png/ref/out-libpng15.txt +++ b/testsuite/png/ref/out-libpng15.txt @@ -12,7 +12,7 @@ Comparing "../oiio-images/oiio-logo-no-alpha.png" and "oiio-logo-no-alpha.png" PASS Reading ../oiio-images/oiio-logo-with-alpha.png ../oiio-images/oiio-logo-with-alpha.png : 135 x 135, 4 channel, uint8 png - SHA-1: 8AED04DCCE8F83B068C537DC0982A42EFBE431B6 + SHA-1: 9F3C517AC714A93C0FE93F8B4B40C68338504DC8 channel list: R, G, B, A Comment: "Created with GIMP" DateTime: "2009:03:26 18:44:26" @@ -27,6 +27,25 @@ exif.png : 64 x 64, 3 channel, uint8 png SHA-1: 7CB41FEA50720B48BE0C145E1473982B23E9AB77 channel list: R, G, B oiio:ColorSpace: "sRGB" + 1 x 1, 4 channel, float png + channel list: R, G, B, A + ResolutionUnit: "inch" + Software: "OpenImageIO 2.4.1.1dev : oiiotool -no-autopremult SLEEP_MM.png -cut 1x1+227+1211 -o kaka.png" + XResolution: 299.999 + YResolution: 299.999 + Exif:ImageHistory: "oiiotool -no-autopremult SLEEP_MM.png -cut 1x1+227+1211 -o kaka.png" + oiio:ColorSpace: "Gamma2.2" + oiio:Gamma: 2.2 + Stats Min: 186 186 186 127 (of 255) + Stats Max: 186 186 186 127 (of 255) + Stats Avg: 186.00 186.00 186.00 127.00 (of 255) + Stats StdDev: 0.00 0.00 0.00 0.00 (of 255) + Stats NanCount: 0 0 0 0 + Stats InfCount: 0 0 0 0 + Stats FiniteCount: 1 1 1 1 + Constant: Yes + Constant Color: 186.00 186.00 186.00 127.00 (of 255) + Monochrome: No smallalpha.png : 1 x 1, 4 channel, uint8 png Pixel (0, 0): 240 108 119 1 (0.94117653 0.42352945 0.4666667 0.003921569) Comparing "test16.png" and "ref/test16.png" diff --git a/testsuite/png/ref/out.txt b/testsuite/png/ref/out.txt index 824b55e0df..3a8ed2f5ab 100644 --- a/testsuite/png/ref/out.txt +++ b/testsuite/png/ref/out.txt @@ -12,7 +12,7 @@ Comparing "../oiio-images/oiio-logo-no-alpha.png" and "oiio-logo-no-alpha.png" PASS Reading ../oiio-images/oiio-logo-with-alpha.png ../oiio-images/oiio-logo-with-alpha.png : 135 x 135, 4 channel, uint8 png - SHA-1: 8AED04DCCE8F83B068C537DC0982A42EFBE431B6 + SHA-1: 9F3C517AC714A93C0FE93F8B4B40C68338504DC8 channel list: R, G, B, A Comment: "Created with GIMP" DateTime: "2009:03:26 18:44:26" @@ -31,6 +31,25 @@ exif.png : 64 x 64, 3 channel, uint8 png Exif:FocalLength: 45.7 (45.7 mm) Exif:WhiteBalance: 0 (auto) oiio:ColorSpace: "sRGB" + 1 x 1, 4 channel, float png + channel list: R, G, B, A + ResolutionUnit: "inch" + Software: "OpenImageIO 2.4.1.1dev : oiiotool -no-autopremult SLEEP_MM.png -cut 1x1+227+1211 -o kaka.png" + XResolution: 299.999 + YResolution: 299.999 + Exif:ImageHistory: "oiiotool -no-autopremult SLEEP_MM.png -cut 1x1+227+1211 -o kaka.png" + oiio:ColorSpace: "Gamma2.2" + oiio:Gamma: 2.2 + Stats Min: 186 186 186 127 (of 255) + Stats Max: 186 186 186 127 (of 255) + Stats Avg: 186.00 186.00 186.00 127.00 (of 255) + Stats StdDev: 0.00 0.00 0.00 0.00 (of 255) + Stats NanCount: 0 0 0 0 + Stats InfCount: 0 0 0 0 + Stats FiniteCount: 1 1 1 1 + Constant: Yes + Constant Color: 186.00 186.00 186.00 127.00 (of 255) + Monochrome: No smallalpha.png : 1 x 1, 4 channel, uint8 png Pixel (0, 0): 240 108 119 1 (0.94117653 0.42352945 0.4666667 0.003921569) Comparing "test16.png" and "ref/test16.png" diff --git a/testsuite/png/run.py b/testsuite/png/run.py index 356629ddcd..33505f9bed 100755 --- a/testsuite/png/run.py +++ b/testsuite/png/run.py @@ -21,6 +21,9 @@ # regression test for 16 bit output bug command += oiiotool ("--pattern fill:topleft=1,0,0,1:topright=0,1,0,1:bottomleft=0,0,1,1:bottomright=1,1,1,1 16x16 4 -d uint16 -o test16.png") +# regression test for wrong gamma correction for partial alpha +command += oiiotool ("src/alphagamma.png --printinfo:stats=1") + # Test high quality alpha deassociation using alpha value close to zero. # This example is inspired by Yafes on the Slack. command += oiiotool ("--pattern fill:color=0.00235,0.00106,0.00117,0.0025 1x1 4 -d uint8 -o smallalpha.png") diff --git a/testsuite/png/src/alphagamma.png b/testsuite/png/src/alphagamma.png new file mode 100644 index 0000000000..a9f79e058e Binary files /dev/null and b/testsuite/png/src/alphagamma.png differ