Remove unreachable TranslateImageLine() colorspace code
TranslateImageLine() has a `bTransMask` parameter that is only true for
CMYK colorspaces. Check it is false for other colorspaces and remove
tests that try to test unreachable code.
Change-Id: Ic9b99581fd7891cd78363aaf8ead40350a46828c
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/117357
Reviewed-by: Thomas Sepez <tsepez@google.com>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfapi/page/cpdf_colorspace.cpp b/core/fpdfapi/page/cpdf_colorspace.cpp
index d458d5e..fafeaf4 100644
--- a/core/fpdfapi/page/cpdf_colorspace.cpp
+++ b/core/fpdfapi/page/cpdf_colorspace.cpp
@@ -653,6 +653,10 @@
int image_width,
int image_height,
bool bTransMask) const {
+ // Only applies to CMYK colorspaces. None of the colorspaces that use this
+ // generic base implementation are CMYK.
+ CHECK(!bTransMask);
+
uint8_t* dest_buf = dest_span.data();
const uint8_t* src_buf = src_span.data();
std::vector<float> src(m_nComponents);
@@ -738,6 +742,8 @@
int image_width,
int image_height,
bool bTransMask) const {
+ CHECK(!bTransMask); // Only applies to CMYK colorspaces.
+
uint8_t* pDestBuf = dest_span.data();
const uint8_t* pSrcBuf = src_span.data();
for (int i = 0; i < pixels; i++) {
@@ -817,28 +823,11 @@
int image_width,
int image_height,
bool bTransMask) const {
+ CHECK(!bTransMask); // Only applies to CMYK colorspaces.
+
uint8_t* pDestBuf = dest_span.data();
const uint8_t* pSrcBuf = src_span.data();
- if (!bTransMask) {
- fxcodec::ReverseRGB(pDestBuf, pSrcBuf, pixels);
- return;
- }
-
- float Cal[3];
- float R;
- float G;
- float B;
- for (int i = 0; i < pixels; i++) {
- Cal[0] = static_cast<float>(pSrcBuf[2]) / 255;
- Cal[1] = static_cast<float>(pSrcBuf[1]) / 255;
- Cal[2] = static_cast<float>(pSrcBuf[0]) / 255;
- GetRGB(Cal, &R, &G, &B);
- pDestBuf[0] = FXSYS_roundf(B * 255);
- pDestBuf[1] = FXSYS_roundf(G * 255);
- pDestBuf[2] = FXSYS_roundf(R * 255);
- pSrcBuf += 3;
- pDestBuf += 3;
- }
+ fxcodec::ReverseRGB(pDestBuf, pSrcBuf, pixels);
}
CPDF_LabCS::CPDF_LabCS() : CPDF_ColorSpace(Family::kLab) {}
@@ -925,6 +914,8 @@
int image_width,
int image_height,
bool bTransMask) const {
+ CHECK(!bTransMask); // Only applies to CMYK colorspaces.
+
uint8_t* pDestBuf = dest_span.data();
const uint8_t* pSrcBuf = src_span.data();
for (int i = 0; i < pixels; i++) {
@@ -1022,6 +1013,8 @@
int image_width,
int image_height,
bool bTransMask) const {
+ CHECK(!bTransMask); // Only applies to CMYK colorspaces.
+
if (profile_->IsSRGB()) {
fxcodec::ReverseRGB(dest_span.data(), src_span.data(), pixels);
return;
diff --git a/core/fpdfapi/page/cpdf_colorspace_unittest.cpp b/core/fpdfapi/page/cpdf_colorspace_unittest.cpp
index d0196fe..759490d 100644
--- a/core/fpdfapi/page/cpdf_colorspace_unittest.cpp
+++ b/core/fpdfapi/page/cpdf_colorspace_unittest.cpp
@@ -19,20 +19,14 @@
uint8_t dst[12];
memset(dst, 0xbd, sizeof(dst));
- pCal->TranslateImageLine(dst, kSrc, 4, 4, 1, true);
- for (size_t i = 0; i < 12; ++i)
- EXPECT_EQ(dst[i], kExpect[i]) << " at " << i;
-
- memset(dst, 0xbd, sizeof(dst));
- pCal->TranslateImageLine(dst, kSrc, 4, 4, 1, false);
+ // `bTransMask` only applies to CYMK colorspaces.
+ pCal->TranslateImageLine(dst, kSrc, 4, 4, 1, /*bTransMask=*/false);
for (size_t i = 0; i < 12; ++i)
EXPECT_EQ(dst[i], kExpect[i]) << " at " << i;
}
TEST(CPDF_CalRGB, TranslateImageLine) {
const uint8_t kSrc[12] = {255, 0, 0, 0, 255, 0, 0, 0, 255, 128, 128, 128};
- const uint8_t kExpectMask[12] = {255, 58, 0, 0, 255, 0,
- 70, 0, 255, 188, 188, 188};
const uint8_t kExpectNomask[12] = {0, 0, 255, 0, 255, 0,
255, 0, 0, 128, 128, 128};
@@ -41,12 +35,8 @@
uint8_t dst[12];
memset(dst, 0xbd, sizeof(dst));
- pCal->TranslateImageLine(dst, kSrc, 4, 4, 1, true);
- for (size_t i = 0; i < 12; ++i)
- EXPECT_EQ(dst[i], kExpectMask[i]) << " at " << i;
-
- memset(dst, 0xbd, sizeof(dst));
- pCal->TranslateImageLine(dst, kSrc, 4, 4, 1, false);
+ // `bTransMask` only applies to CYMK colorspaces.
+ pCal->TranslateImageLine(dst, kSrc, 4, 4, 1, /*bTransMask=*/false);
for (size_t i = 0; i < 12; ++i)
EXPECT_EQ(dst[i], kExpectNomask[i]) << " at " << i;
}
diff --git a/core/fpdfapi/page/cpdf_devicecs.cpp b/core/fpdfapi/page/cpdf_devicecs.cpp
index 6daffb8..d152938 100644
--- a/core/fpdfapi/page/cpdf_devicecs.cpp
+++ b/core/fpdfapi/page/cpdf_devicecs.cpp
@@ -84,6 +84,8 @@
const uint8_t* pSrcBuf = src_span.data();
switch (GetFamily()) {
case Family::kDeviceGray:
+ CHECK(!bTransMask); // Only applies to CMYK colorspaces.
+
for (int i = 0; i < pixels; i++) {
// Compiler can not conclude that src/dest don't overlap, avoid
// duplicate loads.
@@ -94,6 +96,8 @@
}
break;
case Family::kDeviceRGB:
+ CHECK(!bTransMask); // Only applies to CMYK colorspaces.
+
fxcodec::ReverseRGB(pDestBuf, pSrcBuf, pixels);
break;
case Family::kDeviceCMYK:
