Bug 1619946 - Ensure PNG decoder does not recreate sRGB transforms unnecessarily. r=tnikkel

In the PNG decoder, when an image is tagged as sRGB, we create a new
sRGB profile, and a new transform to map that to device space. Creating
a transform is fairly expensive, and we already cache sRGB to device
space transforms in gfxPlatform. This patch ensures that we use the
cached transforms instead. It should be noted that we ignore the intent
from the sRGB tag; nominally we could transform differently as a result
of that, but qcms does not actually implement any of that.

Differential Revision: https://phabricator.services.mozilla.com/D65320

--HG--
extra : moz-landing-system : lando

image/decoders/nsPNGDecoder.cpp

@@ -432,11 +432,8 @@ static void PNGDoGammaCorrection(png_structp png_ptr, png_infop info_ptr) {
 }
 
 // Adapted from http://www.littlecms.com/pngchrm.c example code
-static qcms_profile* PNGGetColorProfile(png_structp png_ptr, png_infop info_ptr,
-                                        int color_type, uint32_t* intent) {
-  qcms_profile* profile = nullptr;
-  *intent = QCMS_INTENT_PERCEPTUAL;  // Our default
-
+uint32_t nsPNGDecoder::ReadColorProfile(png_structp png_ptr, png_infop info_ptr,
+                                        int color_type, bool* sRGBTag) {
   // First try to see if iCCP chunk is present
   if (png_get_valid(png_ptr, info_ptr, PNG_INFO_iCCP)) {
     png_uint_32 profileLen;
@@ -447,9 +444,9 @@ static qcms_profile* PNGGetColorProfile(png_structp png_ptr, png_infop info_ptr,
     png_get_iCCP(png_ptr, info_ptr, &profileName, &compression, &profileData,
                  &profileLen);
 
-    profile = qcms_profile_from_memory((char*)profileData, profileLen);
-    if (profile) {
-      uint32_t profileSpace = qcms_profile_get_color_space(profile);
+    mInProfile = qcms_profile_from_memory((char*)profileData, profileLen);
+    if (mInProfile) {
+      uint32_t profileSpace = qcms_profile_get_color_space(mInProfile);
 
       bool mismatch = false;
       if (color_type & PNG_COLOR_MASK_COLOR) {
@@ -465,31 +462,29 @@ static qcms_profile* PNGGetColorProfile(png_structp png_ptr, png_infop info_ptr,
       }
 
       if (mismatch) {
-        qcms_profile_release(profile);
-        profile = nullptr;
+        qcms_profile_release(mInProfile);
+        mInProfile = nullptr;
       } else {
-        *intent = qcms_profile_get_rendering_intent(profile);
+        return qcms_profile_get_rendering_intent(mInProfile);
       }
     }
   }
 
   // Check sRGB chunk
-  if (!profile && png_get_valid(png_ptr, info_ptr, PNG_INFO_sRGB)) {
-    profile = qcms_profile_sRGB();
+  if (png_get_valid(png_ptr, info_ptr, PNG_INFO_sRGB)) {
+    *sRGBTag = true;
 
-    if (profile) {
-      int fileIntent;
-      png_set_gray_to_rgb(png_ptr);
-      png_get_sRGB(png_ptr, info_ptr, &fileIntent);
-      uint32_t map[] = {
-          QCMS_INTENT_PERCEPTUAL, QCMS_INTENT_RELATIVE_COLORIMETRIC,
-          QCMS_INTENT_SATURATION, QCMS_INTENT_ABSOLUTE_COLORIMETRIC};
-      *intent = map[fileIntent];
-    }
+    int fileIntent;
+    png_set_gray_to_rgb(png_ptr);
+    png_get_sRGB(png_ptr, info_ptr, &fileIntent);
+    uint32_t map[] = {QCMS_INTENT_PERCEPTUAL, QCMS_INTENT_RELATIVE_COLORIMETRIC,
+                      QCMS_INTENT_SATURATION,
+                      QCMS_INTENT_ABSOLUTE_COLORIMETRIC};
+    return map[fileIntent];
   }
 
   // Check gAMA/cHRM chunks
-  if (!profile && png_get_valid(png_ptr, info_ptr, PNG_INFO_gAMA) &&
+  if (png_get_valid(png_ptr, info_ptr, PNG_INFO_gAMA) &&
       png_get_valid(png_ptr, info_ptr, PNG_INFO_cHRM)) {
     qcms_CIE_xyYTRIPLE primaries;
     qcms_CIE_xyY whitePoint;
@@ -503,15 +498,15 @@ static qcms_profile* PNGGetColorProfile(png_structp png_ptr, png_infop info_ptr,
 
     png_get_gAMA(png_ptr, info_ptr, &gammaOfFile);
 
-    profile = qcms_profile_create_rgb_with_gamma(whitePoint, primaries,
-                                                 1.0 / gammaOfFile);
+    mInProfile = qcms_profile_create_rgb_with_gamma(whitePoint, primaries,
+                                                    1.0 / gammaOfFile);
 
-    if (profile) {
+    if (mInProfile) {
       png_set_gray_to_rgb(png_ptr);
     }
   }
 
-  return profile;
+  return QCMS_INTENT_PERCEPTUAL;  // Our default
 }
 
 void nsPNGDecoder::info_callback(png_structp png_ptr, png_infop info_ptr) {
@@ -592,12 +587,12 @@ void nsPNGDecoder::info_callback(png_structp png_ptr, png_infop info_ptr) {
   // fairly expensive to read the profile and create the transform so we should
   // avoid it if not necessary.
   uint32_t intent = -1;
-  uint32_t pIntent;
+  bool sRGBTag = false;
   if (!decoder->IsMetadataDecode()) {
     if (decoder->mCMSMode != eCMSMode_Off) {
       intent = gfxPlatform::GetRenderingIntent();
-      decoder->mInProfile =
-          PNGGetColorProfile(png_ptr, info_ptr, color_type, &pIntent);
+      uint32_t pIntent =
+          decoder->ReadColorProfile(png_ptr, info_ptr, color_type, &sRGBTag);
       // If we're not mandating an intent, use the one from the image.
       if (intent == uint32_t(-1)) {
         intent = pIntent;
@@ -695,7 +690,8 @@ void nsPNGDecoder::info_callback(png_structp png_ptr, png_infop info_ptr) {
     decoder->mTransform = qcms_transform_create(
         decoder->mInProfile, inType, gfxPlatform::GetCMSOutputProfile(),
         outType, (qcms_intent)intent);
-  } else if (decoder->mCMSMode == eCMSMode_All) {
+  } else if ((sRGBTag && decoder->mCMSMode == eCMSMode_TaggedOnly) ||
+             decoder->mCMSMode == eCMSMode_All) {
     // If the transform happens with SurfacePipe, it will be in RGBA if we
     // have an alpha channel, because the swizzle and premultiplication
     // happens after color management. Otherwise it will be in BGRA because

image/decoders/nsPNGDecoder.h

@@ -49,6 +49,9 @@ class nsPNGDecoder : public Decoder {
   nsresult CreateFrame(const FrameInfo& aFrameInfo);
   void EndImageFrame();
 
+  uint32_t ReadColorProfile(png_structp png_ptr, png_infop info_ptr,
+                            int color_type, bool* sRGBTag);
+
   bool HasAlphaChannel() const { return mChannels == 2 || mChannels == 4; }
 
   enum class TransparencyType { eNone, eAlpha, eFrameRect };