Make CFX_DIBitmap::GetScanlineAs() work correctly for 24-bpp bitmaps
Add a test case that demonstrates CFX_DIBitmap allocates memory with a
stride of 12 when the bitmap width is 3 and the format is 24-bpp. The
test case then shows the 12 bytes gets reinterpreted as 4 pixels of 3
bytes each by GetScanlineAs(). Fix GetScanlineAs() to only return the
actual pixels and not the unused space.
Do the same for GetWritableScanlineAs().
Change-Id: I3181bc5dbb5175b91671a995168ff3be21615389
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/123210
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@google.com>
diff --git a/core/fxge/dib/cfx_dibbase.h b/core/fxge/dib/cfx_dibbase.h
index 905d283..66fd6a8 100644
--- a/core/fxge/dib/cfx_dibbase.h
+++ b/core/fxge/dib/cfx_dibbase.h
@@ -45,6 +45,7 @@
static constexpr uint32_t kPaletteSize = 256;
+ // Note that the returned scanline includes unused space at the end, if any.
virtual pdfium::span<const uint8_t> GetScanline(int line) const = 0;
virtual bool SkipToScanline(int line, PauseIndicatorIface* pPause) const;
virtual size_t GetEstimatedImageMemoryBurden() const;
@@ -53,9 +54,12 @@
virtual RetainPtr<const CFX_DIBitmap> RealizeIfNeeded() const;
#endif
+ // Note that the returned scanline does not include unused space at the end,
+ // if any.
template <typename T>
pdfium::span<const T> GetScanlineAs(int line) const {
- return fxcrt::reinterpret_span<const T>(GetScanline(line));
+ return fxcrt::reinterpret_span<const T>(GetScanline(line))
+ .first(GetWidth());
}
int GetWidth() const { return width_; }
diff --git a/core/fxge/dib/cfx_dibitmap.h b/core/fxge/dib/cfx_dibitmap.h
index 53e646f..b98ae8f 100644
--- a/core/fxge/dib/cfx_dibitmap.h
+++ b/core/fxge/dib/cfx_dibitmap.h
@@ -70,6 +70,7 @@
pdfium::make_span(const_cast<uint8_t*>(src.data()), src.size()));
}
+ // Note that the returned scanline includes unused space at the end, if any.
pdfium::span<uint8_t> GetWritableScanline(int line) {
pdfium::span<const uint8_t> src = GetScanline(line);
// SAFETY: const_cast<>() doesn't change size.
@@ -77,9 +78,12 @@
pdfium::make_span(const_cast<uint8_t*>(src.data()), src.size()));
}
+ // Note that the returned scanline does not include unused space at the end,
+ // if any.
template <typename T>
pdfium::span<T> GetWritableScanlineAs(int line) {
- return fxcrt::reinterpret_span<T>(GetWritableScanline(line));
+ return fxcrt::reinterpret_span<T>(GetWritableScanline(line))
+ .first(GetWidth());
}
void TakeOver(RetainPtr<CFX_DIBitmap>&& pSrcBitmap);
diff --git a/core/fxge/dib/cfx_dibitmap_unittest.cpp b/core/fxge/dib/cfx_dibitmap_unittest.cpp
index c46110f..e728060 100644
--- a/core/fxge/dib/cfx_dibitmap_unittest.cpp
+++ b/core/fxge/dib/cfx_dibitmap_unittest.cpp
@@ -118,6 +118,22 @@
FXDIB_Format::k8bppRgb, 0));
}
+TEST(CFXDIBitmapTest, GetScanlineAsWith24Bpp) {
+ auto bitmap = pdfium::MakeRetain<CFX_DIBitmap>();
+ ASSERT_TRUE(bitmap->Create(3, 3, FXDIB_Format::kRgb));
+ EXPECT_EQ(3, bitmap->GetWidth());
+ EXPECT_EQ(12u, bitmap->GetPitch());
+
+ EXPECT_EQ(36u, bitmap->GetBuffer().size());
+ EXPECT_EQ(12u, bitmap->GetScanline(0).size());
+ EXPECT_EQ(3u, bitmap->GetScanlineAs<FX_BGR_STRUCT<uint8_t>>(0).size());
+
+ EXPECT_EQ(36u, bitmap->GetWritableBuffer().size());
+ EXPECT_EQ(12u, bitmap->GetWritableScanline(0).size());
+ EXPECT_EQ(3u,
+ bitmap->GetWritableScanlineAs<FX_BGR_STRUCT<uint8_t>>(0).size());
+}
+
#if defined(PDF_USE_SKIA)
TEST(CFXDIBitmapTest, UnPreMultiplyFromPreMultiplied) {
auto bitmap = pdfium::MakeRetain<CFX_DIBitmap>();
