Add Skia support for 24 bpp FXDIB_Format
Skia doesn't support directly creating SkCanvas from bitmap that's 24
bits-per-pixel. Therefore convert such bitmaps to FXDIB_Format::kArgb
(32 bits-per-pixel) before creating SkCanvas.
Bug: pdfium:1489, pdfium:1953, chromium:1396325
Change-Id: Icdeacc99b161f32d63882118faed7e12459216e3
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/102773
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Nigi <nigi@chromium.org>
diff --git a/core/fxge/dib/cfx_dibitmap.cpp b/core/fxge/dib/cfx_dibitmap.cpp
index 9ce57f6..edd2d01 100644
--- a/core/fxge/dib/cfx_dibitmap.cpp
+++ b/core/fxge/dib/cfx_dibitmap.cpp
@@ -970,7 +970,8 @@
bool CFX_DIBitmap::ConvertFormat(FXDIB_Format dest_format) {
DCHECK(dest_format == FXDIB_Format::k8bppMask ||
dest_format == FXDIB_Format::kArgb ||
- dest_format == kPlatformRGBFormat);
+ dest_format == FXDIB_Format::kRgb32 ||
+ dest_format == FXDIB_Format::kRgb);
if (dest_format == m_Format)
return true;
diff --git a/core/fxge/skia/fx_skia_device.cpp b/core/fxge/skia/fx_skia_device.cpp
index 76e99be..7f8034d 100644
--- a/core/fxge/skia/fx_skia_device.cpp
+++ b/core/fxge/skia/fx_skia_device.cpp
@@ -1343,6 +1343,9 @@
auto driver = pdfium::WrapUnique(
new CFX_SkiaDeviceDriver(std::move(pBitmap), bRgbByteOrder,
std::move(pBackdropBitmap), bGroupKnockout));
+ if (!driver->SkiaCanvas())
+ return nullptr;
+
return driver;
}
@@ -1364,6 +1367,21 @@
color_type = m_pBitmap->IsAlphaFormat() || m_pBitmap->IsMaskFormat()
? kAlpha_8_SkColorType
: kGray_8_SkColorType;
+ } else if (bpp == 24) {
+ DCHECK_EQ(m_pBitmap->GetFormat(), FXDIB_Format::kRgb);
+
+ // Save the input bitmap as `m_pOriginalBitmap` and save its 32 bpp
+ // equivalent at `m_pBitmap` for Skia's internal process.
+ m_pOriginalBitmap = std::move(m_pBitmap);
+ m_pBitmap = pdfium::MakeRetain<CFX_DIBitmap>();
+ if (!m_pBitmap->Copy(m_pOriginalBitmap) ||
+ !m_pBitmap->ConvertFormat(FXDIB_Format::kArgb)) {
+ // Skip creating SkCanvas if we fail to create the 32 bpp bitmap to back
+ // it.
+ return;
+ }
+
+ color_type = Get32BitSkColorType(bRgbByteOrder);
} else {
DCHECK_EQ(bpp, 32);
color_type = Get32BitSkColorType(bRgbByteOrder);
@@ -1394,6 +1412,20 @@
CFX_SkiaDeviceDriver::~CFX_SkiaDeviceDriver() {
Flush();
+
+ // Convert and transfer the internal processed result to the original 24 bpp
+ // bitmap provided by the render device.
+ if (m_pOriginalBitmap && m_pBitmap->ConvertFormat(FXDIB_Format::kRgb)) {
+ int width = m_pOriginalBitmap->GetWidth();
+ int height = m_pOriginalBitmap->GetHeight();
+ DCHECK_EQ(width, m_pBitmap->GetWidth());
+ DCHECK_EQ(height, m_pBitmap->GetHeight());
+ DCHECK_EQ(FXDIB_Format::kRgb, m_pOriginalBitmap->GetFormat());
+ m_pOriginalBitmap->TransferBitmap(/*dest_left=*/0, /*dest_top=*/0, width,
+ height, m_pBitmap, /*src_left=*/0,
+ /*src_top=*/0);
+ }
+
if (!m_pRecorder)
delete m_pCanvas;
}
@@ -2221,6 +2253,9 @@
auto driver =
CFX_SkiaDeviceDriver::Create(std::move(pBitmap), bRgbByteOrder,
std::move(pBackdropBitmap), bGroupKnockout);
+ if (!driver)
+ return false;
+
SetDeviceDriver(std::move(driver));
return true;
}
@@ -2244,6 +2279,9 @@
SetBitmap(pBitmap);
auto driver = CFX_SkiaDeviceDriver::Create(std::move(pBitmap), false,
std::move(pBackdropBitmap), false);
+ if (!driver)
+ return false;
+
SetDeviceDriver(std::move(driver));
return true;
}
diff --git a/core/fxge/skia/fx_skia_device.h b/core/fxge/skia/fx_skia_device.h
index 06ff5ba..dc4269e 100644
--- a/core/fxge/skia/fx_skia_device.h
+++ b/core/fxge/skia/fx_skia_device.h
@@ -167,6 +167,11 @@
RetainPtr<CFX_DIBitmap> m_pBitmap;
RetainPtr<CFX_DIBitmap> m_pBackdropBitmap;
+
+ // The input bitmap passed by the render device. Only used when the input
+ // bitmap is 24 bpp and cannot be directly used as the back of a SkCanvas.
+ RetainPtr<CFX_DIBitmap> m_pOriginalBitmap;
+
SkCanvas* m_pCanvas;
SkPictureRecorder* const m_pRecorder;
std::unique_ptr<SkiaState> m_pCache;
diff --git a/core/fxge/skia/fx_skia_device_embeddertest.cpp b/core/fxge/skia/fx_skia_device_embeddertest.cpp
index dc51c1f..9b4005a 100644
--- a/core/fxge/skia/fx_skia_device_embeddertest.cpp
+++ b/core/fxge/skia/fx_skia_device_embeddertest.cpp
@@ -135,6 +135,7 @@
FPDFBitmap_FillRect(bitmap.get(), 0, 0, kWidth, kHeight, 0x00000000);
RetainPtr<CFX_DIBitmap> pBitmap(CFXDIBitmapFromFPDFBitmap(bitmap.get()));
auto driver = CFX_SkiaDeviceDriver::Create(pBitmap, false, nullptr, false);
+ ASSERT_TRUE(driver);
(*Test)(driver.get(), state);
driver->Flush();
uint32_t pixel = pBitmap->GetPixel(0, 0);
diff --git a/fpdfsdk/fpdf_view_embeddertest.cpp b/fpdfsdk/fpdf_view_embeddertest.cpp
index bc7a506..4462154 100644
--- a/fpdfsdk/fpdf_view_embeddertest.cpp
+++ b/fpdfsdk/fpdf_view_embeddertest.cpp
@@ -1518,26 +1518,25 @@
FPDF_PAGE page = LoadPage(0);
ASSERT_TRUE(page);
+ const char* bgr_checksum = []() {
+ if (CFX_DefaultRenderDevice::SkiaIsDefaultRenderer())
+ return "4d52e5cc1d4a8067bf918b85b232fff0";
+ return "ab6312e04c0d3f4e46fb302a45173d05";
+ }();
+ static constexpr int kBgrStride = 600; // Width of 200 * 24 bits per pixel.
+ TestRenderPageBitmapWithInternalMemory(page, FPDFBitmap_BGR, bgr_checksum);
+ TestRenderPageBitmapWithInternalMemoryAndStride(page, FPDFBitmap_BGR,
+ kBgrStride, bgr_checksum);
+ TestRenderPageBitmapWithExternalMemory(page, FPDFBitmap_BGR, bgr_checksum);
+ TestRenderPageBitmapWithExternalMemoryAndNoStride(page, FPDFBitmap_BGR,
+ bgr_checksum);
+
const char* gray_checksum = []() {
if (CFX_DefaultRenderDevice::SkiaIsDefaultRenderer())
return "3dfe1fc3889123d68e1748fefac65e72";
return "b561c11edc44dc3972125a9b8744fa2f";
}();
- // TODO(crbug.com/pdfium/1489): Add a test for FPDFBitmap_BGR in Skia modes
- // once Skia provides support for BGR24 format.
- if (!CFX_DefaultRenderDevice::SkiaIsDefaultRenderer()) {
- static const char kBgrChecksum[] = "ab6312e04c0d3f4e46fb302a45173d05";
-
- static constexpr int kBgrStride = 600; // Width of 200 * 24 bits per pixel.
- TestRenderPageBitmapWithInternalMemory(page, FPDFBitmap_BGR, kBgrChecksum);
- TestRenderPageBitmapWithInternalMemoryAndStride(page, FPDFBitmap_BGR,
- kBgrStride, kBgrChecksum);
- TestRenderPageBitmapWithExternalMemory(page, FPDFBitmap_BGR, kBgrChecksum);
- TestRenderPageBitmapWithExternalMemoryAndNoStride(page, FPDFBitmap_BGR,
- kBgrChecksum);
- }
-
TestRenderPageBitmapWithInternalMemory(page, FPDFBitmap_Gray, gray_checksum);
static constexpr int kGrayStride = 200; // Width of 200 * 8 bits per pixel.
TestRenderPageBitmapWithInternalMemoryAndStride(page, FPDFBitmap_Gray,
