commit f13f9433868773f73b2e3110459fb3941a41d9f0
author Lei Zhang <thestig@chromium.org> Wed Jul 17 20:04:01 2024 +0000
committer Pdfium LUCI CQ <pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Jul 17 20:04:01 2024 +0000
tree 800d99ad9892162bc72a5f698871cecdf54c15aa
parent 43683d8806460ea1ab618395c36b9352250b8e2f

Simplify format calculation in cpdf_dib.cpp

Take MakeRGBFormat() out of fx_dib.cpp and move it into cpdf_dib.cpp
since that is the only place where it is called from. Then it is easier
to see how parts of MakeRGBFormat() are not reachable due to the few
possible return values from CalculateBitsPerPixel(). Then further
simplify the code by merging CalculateBitsPerPixel() and
MakeRGBFormat() into their only caller.

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

core/fpdfapi/page/cpdf_dib.cpp

diff --git a/core/fpdfapi/page/cpdf_dib.cpp b/core/fpdfapi/page/cpdf_dib.cpp
index 0e11d9b..137053b 100644
--- a/core/fpdfapi/page/cpdf_dib.cpp
+++ b/core/fpdfapi/page/cpdf_dib.cpp
@@ -96,16 +96,6 @@
   return false;
 }
 
-int CalculateBitsPerPixel(uint32_t bpc, uint32_t comps) {
-  uint32_t bpp = bpc * comps;
-  CHECK(bpp);
-  if (bpp == 1)
-    return 1;
-  if (bpp <= 8)
-    return 8;
-  return 24;
-}
-
 CJPX_Decoder::ColorSpaceOption ColorSpaceOptionFromColorSpace(
     CPDF_ColorSpace* pCS) {
   if (!pCS) {
@@ -268,10 +258,18 @@
   if (m_bImageMask) {
     SetMaskProperties();
   } else {
-    if (!m_bpc || !m_nComponents)
+    const uint32_t bpp = m_bpc * m_nComponents;
+    if (bpp == 0) {
       return false;
+    }
 
-    SetFormat(MakeRGBFormat(CalculateBitsPerPixel(m_bpc, m_nComponents)));
+    if (bpp == 1) {
+      SetFormat(FXDIB_Format::k1bppRgb);
+    } else if (bpp <= 8) {
+      SetFormat(FXDIB_Format::k8bppRgb);
+    } else {
+      SetFormat(FXDIB_Format::kRgb);
+    }
   }
 
   std::optional<uint32_t> pitch = fxge::CalculatePitch32(GetBPP(), GetWidth());

core/fxge/dib/fx_dib.cpp

diff --git a/core/fxge/dib/fx_dib.cpp b/core/fxge/dib/fx_dib.cpp
index d43ed3a..315e958 100644
--- a/core/fxge/dib/fx_dib.cpp
+++ b/core/fxge/dib/fx_dib.cpp
@@ -39,21 +39,6 @@
 static_assert(std::is_aggregate_v<FX_BGRA_STRUCT<float>>);
 static_assert(std::is_aggregate_v<FX_CMYK_STRUCT<float>>);
 
-FXDIB_Format MakeRGBFormat(int bpp) {
-  switch (bpp) {
-    case 1:
-      return FXDIB_Format::k1bppRgb;
-    case 8:
-      return FXDIB_Format::k8bppRgb;
-    case 24:
-      return FXDIB_Format::kRgb;
-    case 32:
-      return FXDIB_Format::kRgb32;
-    default:
-      return FXDIB_Format::kInvalid;
-  }
-}
-
 FXDIB_ResampleOptions::FXDIB_ResampleOptions() = default;
 
 bool FXDIB_ResampleOptions::HasAnyOptions() const {

core/fxge/dib/fx_dib.h

diff --git a/core/fxge/dib/fx_dib.h b/core/fxge/dib/fx_dib.h
index d590241..59d1248 100644
--- a/core/fxge/dib/fx_dib.h
+++ b/core/fxge/dib/fx_dib.h
@@ -159,8 +159,6 @@
   return !!(static_cast<uint16_t>(format) & 0x100);
 }
 
-FXDIB_Format MakeRGBFormat(int bpp);
-
 FX_BGRA_STRUCT<uint8_t> ArgbToBGRAStruct(FX_ARGB argb);
 
 // Ignores alpha.