Refactor code into GetJpxDecodeAction().
This moves some JPEG2000 colorspace handling code out of
CPDF_DIBBase::LoadJpxBitmap() and into a standalone function. Instead of
directly manipulating CPDF_DIBBase state, the new GetJpxDecodeAction()
function returns an enum value to tell CPDF_DIBBase what to do.
This will likely make fixing https://crbug.com/1012369 easier, because
the fix will try to handle JPEG2000 vs. PDF colorspace mismatches, and
the code that handles those cases may be needed for multiple conditions.
Bug: chromium:1012369
Change-Id: I1fc78d8d524697774122d53903c2b7db59d8095b
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/61810
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fpdfapi/page/cpdf_dib.cpp b/core/fpdfapi/page/cpdf_dib.cpp
index 1a1c8f1..87845b7 100644
--- a/core/fpdfapi/page/cpdf_dib.cpp
+++ b/core/fpdfapi/page/cpdf_dib.cpp
@@ -102,6 +102,41 @@
return CJPX_Decoder::kNormalColorSpace;
}
+enum class JpxDecodeAction {
+ kFail,
+ kDoNothing,
+ kUseRgb,
+ kUseCmyk,
+};
+
+JpxDecodeAction GetJpxDecodeAction(uint32_t jpx_components,
+ const CPDF_ColorSpace* pdf_colorspace) {
+ if (pdf_colorspace) {
+ // Make sure the JPX image and the PDF colorspace agree on the number of
+ // components.
+ if (jpx_components != pdf_colorspace->CountComponents())
+ return JpxDecodeAction::kFail;
+
+ if (pdf_colorspace == CPDF_ColorSpace::GetStockCS(PDFCS_DEVICERGB))
+ return JpxDecodeAction::kUseRgb;
+
+ return JpxDecodeAction::kDoNothing;
+ }
+
+ // Cases where the PDF did not provide a colorspace.
+ // Choose how to decode based on the number of components in the JPX image.
+ switch (jpx_components) {
+ case 3:
+ return JpxDecodeAction::kUseRgb;
+
+ case 4:
+ return JpxDecodeAction::kUseCmyk;
+
+ default:
+ return JpxDecodeAction::kDoNothing;
+ }
+}
+
} // namespace
CPDF_DIB::CPDF_DIB() = default;
@@ -382,8 +417,12 @@
if (!m_pColorSpace)
return false;
- m_Family = m_pColorSpace->GetFamily();
+ // If the checks above failed to find a colorspace, and the next line to set
+ // |m_nComponents| does not get reached, then a decoder can try to set
+ // |m_nComponents| based on the number of components in the image being
+ // decoded.
m_nComponents = m_pColorSpace->CountComponents();
+ m_Family = m_pColorSpace->GetFamily();
if (m_Family == PDFCS_ICCBASED && pCSObj->IsName()) {
ByteString cs = pCSObj->GetString();
if (cs == "DeviceGray")
@@ -595,21 +634,31 @@
if (static_cast<int>(width) < m_Width || static_cast<int>(height) < m_Height)
return nullptr;
- bool bSwapRGB = false;
- if (m_pColorSpace) {
- if (components != m_pColorSpace->CountComponents())
+ RetainPtr<CPDF_ColorSpace> original_colorspace = m_pColorSpace;
+ bool swap_rgb = false;
+ switch (GetJpxDecodeAction(components, m_pColorSpace.Get())) {
+ case JpxDecodeAction::kFail:
return nullptr;
- if (m_pColorSpace == CPDF_ColorSpace::GetStockCS(PDFCS_DEVICERGB)) {
- bSwapRGB = true;
+ case JpxDecodeAction::kDoNothing:
+ break;
+
+ case JpxDecodeAction::kUseRgb:
+ swap_rgb = true;
m_pColorSpace = nullptr;
- }
- } else {
- if (components == 3) {
- bSwapRGB = true;
- } else if (components == 4) {
+ break;
+
+ case JpxDecodeAction::kUseCmyk:
m_pColorSpace = CPDF_ColorSpace::GetStockCS(PDFCS_DEVICECMYK);
- }
+ break;
+ }
+
+ // If |original_colorspace| exists, then LoadColorInfo() already set
+ // |m_nComponents|.
+ if (original_colorspace) {
+ DCHECK_NE(0, m_nComponents);
+ } else {
+ DCHECK_EQ(0, m_nComponents);
m_nComponents = components;
}
@@ -633,7 +682,7 @@
// Fill |output_offsets| with 0, 1, ... N.
std::vector<uint8_t> output_offsets(components);
std::iota(output_offsets.begin(), output_offsets.end(), 0);
- if (bSwapRGB)
+ if (swap_rgb)
std::swap(output_offsets[0], output_offsets[2]);
if (!decoder->Decode(pCachedBitmap->GetBuffer(), pCachedBitmap->GetPitch(),
output_offsets)) {
