Tidy JBig2_Image.cpp
Add checked/unchecked GetLine(y) methods and use them.
Introduce BIT_INDEX_TO_ALIGNED_BYTE() to de-mystify some shifting.
Move local declarations to spot of use.
Remove spurious Fill(), as we initialize to 0s.
Initialize members in header where possible.
Add unit tests.
Change-Id: I41ccb91b57320dbc790fd0f680f6d98571280343
Reviewed-on: https://pdfium-review.googlesource.com/39370
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/core/fxcodec/jbig2/JBig2_Image.cpp b/core/fxcodec/jbig2/JBig2_Image.cpp
index 6c61c78..f66780a 100644
--- a/core/fxcodec/jbig2/JBig2_Image.cpp
+++ b/core/fxcodec/jbig2/JBig2_Image.cpp
@@ -16,6 +16,7 @@
#include "core/fxcrt/fx_memory.h"
#include "core/fxcrt/fx_safe_types.h"
#include "third_party/base/ptr_util.h"
+#include "third_party/base/stl_util.h"
#define JBIG2_GETDWORD(buf) \
((static_cast<uint32_t>((buf)[0]) << 24) | \
@@ -29,6 +30,8 @@
(buf)[2] = static_cast<uint8_t>((val) >> 8), \
(buf)[3] = static_cast<uint8_t>((val) >> 0))
+#define BIT_INDEX_TO_ALIGNED_BYTE(x) (((x) >> 5) << 2)
+
namespace {
const int kMaxImagePixels = INT_MAX - 31;
@@ -36,8 +39,7 @@
} // namespace
-CJBig2_Image::CJBig2_Image(int32_t w, int32_t h)
- : m_pData(nullptr), m_nWidth(0), m_nHeight(0), m_nStride(0) {
+CJBig2_Image::CJBig2_Image(int32_t w, int32_t h) {
if (w <= 0 || h <= 0 || w > kMaxImagePixels)
return;
@@ -52,9 +54,15 @@
FX_Alloc2D(uint8_t, m_nStride, m_nHeight)));
}
-CJBig2_Image::CJBig2_Image(int32_t w, int32_t h, int32_t stride, uint8_t* pBuf)
- : m_pData(nullptr), m_nWidth(0), m_nHeight(0), m_nStride(0) {
- if (w < 0 || h < 0 || stride < 0 || stride > kMaxImageBytes)
+CJBig2_Image::CJBig2_Image(int32_t w,
+ int32_t h,
+ int32_t stride,
+ uint8_t* pBuf) {
+ if (w < 0 || h < 0)
+ return;
+
+ // Stride must be word-aligned.
+ if (stride < 0 || stride > kMaxImageBytes || stride % 4 != 0)
return;
int32_t stride_pixels = 8 * stride;
@@ -68,8 +76,7 @@
}
CJBig2_Image::CJBig2_Image(const CJBig2_Image& other)
- : m_pData(nullptr),
- m_nWidth(other.m_nWidth),
+ : m_nWidth(other.m_nWidth),
m_nHeight(other.m_nHeight),
m_nStride(other.m_nStride) {
if (other.m_pData) {
@@ -117,6 +124,10 @@
data()[m] &= ~n;
}
+uint8_t* CJBig2_Image::GetLine(int32_t y) const {
+ return (y >= 0 && y < m_nHeight) ? GetLineUnsafe(y) : nullptr;
+}
+
void CJBig2_Image::CopyLine(int32_t hTo, int32_t hFrom) {
if (!m_pData)
return;
@@ -170,60 +181,40 @@
int32_t y,
int32_t w,
int32_t h) {
- int32_t m;
- int32_t n;
- int32_t j;
- uint8_t* pLineSrc;
- uint8_t* pLineDst;
- uint32_t wTmp;
- uint8_t* pSrc;
- uint8_t* pSrcEnd;
- uint8_t* pDst;
- uint8_t* pDstEnd;
- if (w == 0 || h == 0)
- return nullptr;
-
auto pImage = pdfium::MakeUnique<CJBig2_Image>(w, h);
- if (!m_pData) {
- pImage->Fill(0);
+ if (!pImage->data() || !m_pData)
+ return pImage;
+
+ if (x < 0 || x >= m_nWidth || y < 0 || y >= m_nHeight)
+ return pImage;
+
+ int32_t m = BIT_INDEX_TO_ALIGNED_BYTE(x);
+ int32_t n = x & 31;
+ int32_t bytes_to_copy = std::min(pImage->m_nStride, m_nStride - m);
+ int32_t lines_to_copy = std::min(pImage->m_nHeight, m_nHeight - y);
+
+ // Fast case when DWORD-aligned.
+ if (n == 0) {
+ for (int32_t j = 0; j < lines_to_copy; j++) {
+ const uint8_t* pLineSrc = GetLineUnsafe(y + j);
+ uint8_t* pLineDst = pImage->GetLineUnsafe(j);
+ memcpy(pLineDst, pLineSrc + m, bytes_to_copy);
+ }
return pImage;
}
- if (!pImage->m_pData)
- return pImage;
- pLineSrc = data() + m_nStride * y;
- pLineDst = pImage->data();
- m = (x >> 5) << 2;
- n = x & 31;
- if (n == 0) {
- for (j = 0; j < h; j++) {
- pSrc = pLineSrc + m;
- pSrcEnd = pLineSrc + m_nStride;
- pDst = pLineDst;
- pDstEnd = pLineDst + pImage->m_nStride;
- for (; pDst < pDstEnd; pSrc += 4, pDst += 4) {
- *((uint32_t*)pDst) = *((uint32_t*)pSrc);
- }
- pLineSrc += m_nStride;
- pLineDst += pImage->m_nStride;
- }
- } else {
- for (j = 0; j < h; j++) {
- pSrc = pLineSrc + m;
- pSrcEnd = pLineSrc + m_nStride;
- pDst = pLineDst;
- pDstEnd = pLineDst + pImage->m_nStride;
- for (; pDst < pDstEnd; pSrc += 4, pDst += 4) {
- if (pSrc + 4 < pSrcEnd) {
- wTmp = (JBIG2_GETDWORD(pSrc) << n) |
- (JBIG2_GETDWORD(pSrc + 4) >> (32 - n));
- } else {
- wTmp = JBIG2_GETDWORD(pSrc) << n;
- }
- JBIG2_PUTDWORD(pDst, wTmp);
- }
- pLineSrc += m_nStride;
- pLineDst += pImage->m_nStride;
+ // Normal slow case.
+ for (int32_t j = 0; j < lines_to_copy; j++) {
+ const uint8_t* pLineSrc = GetLineUnsafe(y + j);
+ uint8_t* pLineDst = pImage->GetLineUnsafe(j);
+ const uint8_t* pSrc = pLineSrc + m;
+ const uint8_t* pSrcEnd = pLineSrc + m_nStride;
+ uint8_t* pDstEnd = pLineDst + bytes_to_copy;
+ for (uint8_t *pDst = pLineDst; pDst < pDstEnd; pSrc += 4, pDst += 4) {
+ uint32_t wTmp = JBIG2_GETDWORD(pSrc) << n;
+ if (pSrc + 4 < pSrcEnd)
+ wTmp |= (JBIG2_GETDWORD(pSrc + 4) >> (32 - n));
+ JBIG2_PUTDWORD(pDst, wTmp);
}
}
return pImage;
@@ -382,7 +373,6 @@
} else {
uint8_t* sp = nullptr;
uint8_t* dp = nullptr;
-
if (s1 > d1) {
uint32_t shift1 = s1 - d1;
uint32_t shift2 = 32 - shift1;
diff --git a/core/fxcodec/jbig2/JBig2_Image.h b/core/fxcodec/jbig2/JBig2_Image.h
index 3cf28b0..b1b1489 100644
--- a/core/fxcodec/jbig2/JBig2_Image.h
+++ b/core/fxcodec/jbig2/JBig2_Image.h
@@ -34,11 +34,14 @@
int32_t width() const { return m_nWidth; }
int32_t height() const { return m_nHeight; }
int32_t stride() const { return m_nStride; }
+
uint8_t* data() const { return m_pData.Get(); }
int GetPixel(int32_t x, int32_t y) const;
void SetPixel(int32_t x, int32_t y, int bVal);
+ uint8_t* GetLine(int32_t y) const;
+ uint8_t* GetLineUnsafe(int32_t y) const { return data() + y * m_nStride; }
void CopyLine(int32_t hTo, int32_t hFrom);
void Fill(bool v);
@@ -74,9 +77,9 @@
const FX_RECT& rtSrc);
MaybeOwned<uint8_t, FxFreeDeleter> m_pData;
- int32_t m_nWidth; // 1-bit pixels
- int32_t m_nHeight; // lines
- int32_t m_nStride; // bytes
+ int32_t m_nWidth = 0; // 1-bit pixels
+ int32_t m_nHeight = 0; // lines
+ int32_t m_nStride = 0; // bytes, must be multiple of 4.
};
#endif // CORE_FXCODEC_JBIG2_JBIG2_IMAGE_H_
diff --git a/core/fxcodec/jbig2/JBig2_Image_unittest.cpp b/core/fxcodec/jbig2/JBig2_Image_unittest.cpp
index bebfb6b..2b4d897 100644
--- a/core/fxcodec/jbig2/JBig2_Image_unittest.cpp
+++ b/core/fxcodec/jbig2/JBig2_Image_unittest.cpp
@@ -8,26 +8,75 @@
#include "core/fxcodec/jbig2/JBig2_Image.h"
#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/base/ptr_util.h"
namespace {
const int32_t kWidthPixels = 80;
const int32_t kWidthBytes = 10;
-const int32_t kStrideBytes = kWidthBytes + 1; // For testing stride != width.
+const int32_t kStrideBytes = kWidthBytes + 2; // For testing stride != width.
const int32_t kHeightLines = 20;
const int32_t kLargerHeightLines = 100;
const int32_t kTooLargeHeightLines = 40000000;
+void CheckImageEq(CJBig2_Image* img1, CJBig2_Image* img2, int line) {
+ EXPECT_EQ(img1->width(), img2->width());
+ EXPECT_EQ(img1->height(), img2->height());
+ for (int32_t y = 0; y < img1->height(); ++y) {
+ for (int32_t x = 0; x < img1->width(); ++x) {
+ EXPECT_EQ(img1->GetPixel(x, y), img2->GetPixel(x, y))
+ << " at " << x << " " << y << " actual line " << line;
+ }
+ }
+}
+
} // namespace
+TEST(fxcodec, EmptyImage) {
+ CJBig2_Image empty(0, 0);
+ EXPECT_EQ(empty.width(), 0);
+ EXPECT_EQ(empty.height(), 0);
+
+ // Out-of-bounds SetPixel() is silent no-op.
+ empty.SetPixel(0, 0, true);
+ empty.SetPixel(1, 1, true);
+
+ // Out-of-bounds GetPixel returns 0.
+ EXPECT_EQ(empty.GetPixel(0, 0), 0);
+ EXPECT_EQ(empty.GetPixel(1, 1), 0);
+
+ // Out-of-bounds GetLine() returns null.
+ EXPECT_EQ(empty.GetLine(0), nullptr);
+ EXPECT_EQ(empty.GetLine(1), nullptr);
+}
+
TEST(fxcodec, JBig2ImageCreate) {
CJBig2_Image img(kWidthPixels, kHeightLines);
- img.SetPixel(0, 0, true);
- img.SetPixel(kWidthPixels - 1, kHeightLines - 1, false);
EXPECT_EQ(kWidthPixels, img.width());
EXPECT_EQ(kHeightLines, img.height());
- EXPECT_TRUE(img.GetPixel(0, 0));
- EXPECT_FALSE(img.GetPixel(kWidthPixels - 1, kHeightLines - 1));
+ EXPECT_EQ(0, img.GetPixel(0, 0));
+ EXPECT_EQ(0, img.GetLine(0)[0]);
+ EXPECT_EQ(0, img.GetPixel(kWidthPixels - 1, kHeightLines - 1));
+ EXPECT_EQ(0, img.GetLine(kHeightLines - 1)[kWidthBytes - 1]);
+
+ img.SetPixel(0, 0, true);
+ img.SetPixel(kWidthPixels - 1, kHeightLines - 1, true);
+ EXPECT_EQ(1, img.GetPixel(0, 0));
+ EXPECT_EQ(1, img.GetPixel(kWidthPixels - 1, kHeightLines - 1));
+ EXPECT_EQ(0x80, img.GetLine(0)[0]);
+ EXPECT_EQ(0x01, img.GetLine(kHeightLines - 1)[kWidthBytes - 1]);
+
+ // Out-of-bounds SetPixel() is silent no-op.
+ img.SetPixel(-1, 1, true);
+ img.SetPixel(kWidthPixels, kHeightLines, true);
+
+ // Out-of-bounds GetPixel returns 0.
+ EXPECT_EQ(0, img.GetPixel(-1, -1));
+ EXPECT_EQ(0, img.GetPixel(kWidthPixels, kHeightLines));
+
+ // Out-of-bounds GetLine() returns null.
+ EXPECT_EQ(nullptr, img.GetLine(-1));
+ EXPECT_EQ(nullptr, img.GetLine(kHeightLines));
}
TEST(fxcodec, JBig2ImageCreateTooBig) {
@@ -56,6 +105,14 @@
EXPECT_EQ(nullptr, img.data());
}
+TEST(fxcodec, JBig2ImageCreateExternalBadStride) {
+ uint8_t buf[kHeightLines * kStrideBytes];
+ CJBig2_Image img(kWidthPixels, kTooLargeHeightLines, kStrideBytes - 1, buf);
+ EXPECT_EQ(0, img.width());
+ EXPECT_EQ(0, img.height());
+ EXPECT_EQ(nullptr, img.data());
+}
+
TEST(fxcodec, JBig2ImageExpand) {
CJBig2_Image img(kWidthPixels, kHeightLines);
img.SetPixel(0, 0, true);
@@ -103,3 +160,140 @@
EXPECT_TRUE(img.GetPixel(0, 0));
EXPECT_FALSE(img.GetPixel(kWidthPixels - 1, kHeightLines - 1));
}
+
+TEST(fxcodec, JBig2EmptyImage) {
+ auto empty = pdfium::MakeUnique<CJBig2_Image>(0, 0);
+
+ // Empty subimage.
+ auto sub1 = empty->SubImage(0, 0, 0, 0);
+ EXPECT_EQ(sub1->width(), 0);
+ EXPECT_EQ(sub1->height(), 0);
+
+ // Larger dimensions are zero-padded.
+ auto sub2 = empty->SubImage(0, 0, 1, 1);
+ EXPECT_EQ(1, sub2->width());
+ EXPECT_EQ(1, sub2->height());
+ EXPECT_EQ(0, sub2->GetPixel(0, 0));
+
+ // Bad dimensions give an empty image.
+ sub2 = empty->SubImage(0, 0, -1, -1);
+ EXPECT_EQ(sub2->width(), 0);
+ EXPECT_EQ(sub2->height(), 0);
+
+ // Bad offsets zero pad the image.
+ auto sub3 = empty->SubImage(-1, -1, 2, 2);
+ EXPECT_EQ(sub3->width(), 2);
+ EXPECT_EQ(sub3->height(), 2);
+
+ // Bad dimensions and bad offsets give an empty image.
+ sub3 = empty->SubImage(-1, -1, -100, -100);
+ EXPECT_EQ(sub3->width(), 0);
+ EXPECT_EQ(sub3->height(), 0);
+}
+
+TEST(fxcodec, JBig2SubImage) {
+ // 1-px wide rectangle in image.
+ uint8_t pattern[5][8] = {
+ {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
+ {0x00, 0x01, 0xff, 0x80, 0x00, 0x00, 0x00, 0x00},
+ {0x00, 0x01, 0x00, 0x80, 0x00, 0x00, 0x00, 0x00},
+ {0x00, 0x01, 0xff, 0x80, 0x00, 0x00, 0x00, 0x00},
+ {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
+ };
+
+ // 1-px wide rectangle in image, offset 2 in x.
+ uint8_t pattern20[5][8] = {
+ {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
+ {0x00, 0x07, 0xfe, 0x00, 0x00, 0x00, 0x00, 0x00},
+ {0x00, 0x04, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00},
+ {0x00, 0x07, 0xfe, 0x00, 0x00, 0x00, 0x00, 0x00},
+ {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
+ };
+
+ // 1-px wide rectangle in image, offset 2 in x and y, padded.
+ uint8_t pattern22[5][8] = {
+ {0x00, 0x04, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00},
+ {0x00, 0x07, 0xfe, 0x00, 0x00, 0x00, 0x00, 0x00},
+ {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
+ {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
+ {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
+ };
+
+ // Image size a nice clean power of two.
+ auto img32 = pdfium::MakeUnique<CJBig2_Image>(
+ 32, 5, 8, reinterpret_cast<uint8_t*>(pattern));
+
+ // Image size not a nice clean value.
+ auto img37 = pdfium::MakeUnique<CJBig2_Image>(
+ 37, 5, 8, reinterpret_cast<uint8_t*>(pattern));
+
+ // Expected results to check against.
+ auto expected20 = pdfium::MakeUnique<CJBig2_Image>(
+ 30, 5, 8, reinterpret_cast<uint8_t*>(pattern20));
+
+ auto expected22 = pdfium::MakeUnique<CJBig2_Image>(
+ 30, 5, 8, reinterpret_cast<uint8_t*>(pattern22));
+
+ auto expected_zeros = pdfium::MakeUnique<CJBig2_Image>(32, 5);
+
+ // Empty subimage.
+ auto sub = img32->SubImage(0, 0, 0, 0);
+ EXPECT_EQ(sub->width(), 0);
+ EXPECT_EQ(sub->height(), 0);
+
+ // Full sub-image.
+ sub = img32->SubImage(0, 0, 32, 5);
+ EXPECT_EQ(sub->width(), 32);
+ EXPECT_EQ(sub->height(), 5);
+ CheckImageEq(img32.get(), sub.get(), __LINE__);
+
+ sub = img37->SubImage(0, 0, 32, 5);
+ EXPECT_EQ(sub->width(), 32);
+ EXPECT_EQ(sub->height(), 5);
+ CheckImageEq(img32.get(), sub.get(), __LINE__);
+
+ // Actual bit manipulations.
+ sub = img32->SubImage(2, 0, 30, 5);
+ CheckImageEq(expected20.get(), sub.get(), __LINE__);
+
+ sub = img37->SubImage(2, 2, 30, 5);
+ CheckImageEq(expected22.get(), sub.get(), __LINE__);
+
+ // Aligned Sub-image including cruft in stride beyond width.
+ sub = img37->SubImage(32, 0, 32, 5);
+ CheckImageEq(expected_zeros.get(), sub.get(), __LINE__);
+
+ // Sub-image waaaaay beyond width.
+ sub = img37->SubImage(2000, 0, 32, 5);
+ CheckImageEq(expected_zeros.get(), sub.get(), __LINE__);
+
+ // Sub-image waaaaay beyond height.
+ sub = img37->SubImage(0, 2000, 32, 5);
+ CheckImageEq(expected_zeros.get(), sub.get(), __LINE__);
+
+ // Sub-image with negative x offset.
+ sub = img37->SubImage(-1, 0, 32, 5);
+ CheckImageEq(expected_zeros.get(), sub.get(), __LINE__);
+
+ // Sub-image with negative y offset.
+ sub = img37->SubImage(0, -1, 32, 5);
+ CheckImageEq(expected_zeros.get(), sub.get(), __LINE__);
+
+ // Sub-image with negative width.
+ sub = img37->SubImage(-1, 0, 32, 5);
+ CheckImageEq(expected_zeros.get(), sub.get(), __LINE__);
+
+ // Sub-image with negative height.
+ sub = img37->SubImage(0, -1, 32, 5);
+ CheckImageEq(expected_zeros.get(), sub.get(), __LINE__);
+
+ // Sub-image wider than original.
+ sub = img37->SubImage(0, 0, 128, 5);
+ EXPECT_EQ(128, sub->width());
+ EXPECT_EQ(5, sub->height());
+
+ // Sub-image higher than original.
+ sub = img37->SubImage(0, 0, 32, 40);
+ EXPECT_EQ(32, sub->width());
+ EXPECT_EQ(40, sub->height());
+}
