commit b5bc2e9294e634851cb1e8d3ee1f8b7c711ce358
author Lei Zhang <thestig@chromium.org> Mon Sep 18 21:25:39 2023 +0000
committer Pdfium LUCI CQ <pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com> Mon Sep 18 21:25:39 2023 +0000
tree 3e20aeb103025ca6cc9a5282f2ba6afca7e59828
parent 372d25d7d17545b518cf7cfa55727fc9edb83aa9

Refine FPDFImageObj_GetBitmap() behavior

Instead of always converting 1 bpp / 8 bpp bitmaps to a color bitmap,
try to convert to a 8 bpp grayscale bitmap when possible. Use knowledge
of how the format conversion works to choose the appropriate conversion
operation. Add some CHECKs to backup those assumptions and make sure the
returned bitmap does not have a color palette.

Bug: pdfium:1190
Change-Id: I0c98d484b7de65926d7bde74d6ee295bfc2c5bb2
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/111950
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>

fpdfsdk/fpdf_edit_embeddertest.cpp

diff --git a/fpdfsdk/fpdf_edit_embeddertest.cpp b/fpdfsdk/fpdf_edit_embeddertest.cpp
index 02edaf0..587e769 100644
--- a/fpdfsdk/fpdf_edit_embeddertest.cpp
+++ b/fpdfsdk/fpdf_edit_embeddertest.cpp
@@ -4166,8 +4166,8 @@
   {
     ScopedFPDFBitmap bitmap(FPDFImageObj_GetBitmap(obj));
     ASSERT_TRUE(bitmap);
-    EXPECT_EQ(FPDFBitmap_BGR, FPDFBitmap_GetFormat(bitmap.get()));
-    CompareBitmap(bitmap.get(), 1152, 720, "0b67a5d69f3f1052abddd5cdd882a126");
+    EXPECT_EQ(FPDFBitmap_Gray, FPDFBitmap_GetFormat(bitmap.get()));
+    CompareBitmap(bitmap.get(), 1152, 720, "3f6a48e2b3e91b799bf34567f55cb4de");
   }
 
   UnloadPage(page);

fpdfsdk/fpdf_editimg.cpp

diff --git a/fpdfsdk/fpdf_editimg.cpp b/fpdfsdk/fpdf_editimg.cpp
index f26c1eb..a5b8dad 100644
--- a/fpdfsdk/fpdf_editimg.cpp
+++ b/fpdfsdk/fpdf_editimg.cpp
@@ -200,28 +200,47 @@
     return nullptr;
 
   // If the source image has a representation of 1 bit per pixel, or if the
-  // source image has a color palette, convert it to a color representation if
-  // needed to get rid of the palette, as there is no public API to access to
-  // palette.
+  // source image has a color palette, convert it to a representation that does
+  // not have a color palette, as there is no public API to access the palette.
   //
   // Otherwise, convert the source image to a bitmap directly,
   // retaining its color representation.
   //
   // Only return FPDF_BITMAPs in formats that FPDFBitmap_CreateEx() would
   // return.
-  RetainPtr<CFX_DIBitmap> pBitmap;
+  enum class ConversionOp {
+    kRealize,
+    kConvertTo8bppRgb,
+    kConvertToRgb,
+  };
+
+  ConversionOp op;
   switch (pSource->GetFormat()) {
     case FXDIB_Format::k1bppMask:
-    case FXDIB_Format::k1bppRgb:
     case FXDIB_Format::k8bppMask:
+      // Masks do not have palettes, so they can be safely converted to
+      // `FXDIB_Format::k8bppRgb`.
+      CHECK(!pSource->HasPalette());
+      op = ConversionOp::kConvertTo8bppRgb;
+      break;
+    case FXDIB_Format::k1bppRgb:
+      // If there is a palette, then convert to `FXDIB_Format::kRgb` to avoid
+      // creating a bitmap with a palette.
+      op = pSource->HasPalette() ? ConversionOp::kConvertToRgb
+                                 : ConversionOp::kConvertTo8bppRgb;
+      break;
     case FXDIB_Format::k8bppRgb:
-      pBitmap = pSource->ConvertTo(FXDIB_Format::kRgb);
+      // If there is a palette, then convert to `FXDIB_Format::kRgb` to avoid
+      // creating a bitmap with a palette.
+      op = pSource->HasPalette() ? ConversionOp::kConvertToRgb
+                                 : ConversionOp::kRealize;
       break;
 
     case FXDIB_Format::kArgb:
     case FXDIB_Format::kRgb:
     case FXDIB_Format::kRgb32:
-      pBitmap = pSource->Realize();
+      CHECK(!pSource->HasPalette());
+      op = ConversionOp::kRealize;
       break;
 
     case FXDIB_Format::kInvalid: {
@@ -229,6 +248,22 @@
     }
   }
 
+  RetainPtr<CFX_DIBitmap> pBitmap;
+  switch (op) {
+    case ConversionOp::kRealize:
+      pBitmap = pSource->Realize();
+      break;
+    case ConversionOp::kConvertTo8bppRgb:
+      pBitmap = pSource->ConvertTo(FXDIB_Format::k8bppRgb);
+      break;
+    case ConversionOp::kConvertToRgb:
+      pBitmap = pSource->ConvertTo(FXDIB_Format::kRgb);
+      break;
+  }
+  if (pBitmap) {
+    CHECK(!pBitmap->HasPalette());
+  }
+
   return FPDFBitmapFromCFXDIBitmap(pBitmap.Leak());
 }