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,