Add special FPDFBitmap_FillRect() handling for 24 and 32 BPP bitmaps
If Skia were to truly use pre-multiplied bitmaps internally, then the
current FPDFBitmap_FillRect() implementation would take an
un-premultiplied bitmap, pre-multiply it, let Skia fill it, and
un-premultiply it. Avoid this extra work by changing
FPDFBitmap_FillRect() to write to 32 BPP bitmaps directly.
Similarly, handle filling 24 BPP bitmaps directly in
FPDFBitmap_FillRect() to avoid a roundtrip conversion to 32 BPP and
back.
This also improves the rendering in an embedder test that uses
FPDFBitmap_FillRect(). There, the current Skia-specific test
expectations are for an image that is a drawn a bit too dark.
Bug: 42271020
Change-Id: Id156780f3ea4aa7479dc933b2da5ca192b2e27a1
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/122911
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@google.com>
diff --git a/fpdfsdk/fpdf_annot_embeddertest.cpp b/fpdfsdk/fpdf_annot_embeddertest.cpp
index d7c53da..dc4dd1d 100644
--- a/fpdfsdk/fpdf_annot_embeddertest.cpp
+++ b/fpdfsdk/fpdf_annot_embeddertest.cpp
@@ -1387,11 +1387,11 @@
const char* md5_new_image = []() {
if (CFX_DefaultRenderDevice::UseSkiaRenderer()) {
#if BUILDFLAG(IS_WIN)
- return "38c4bdbb9fd3fcc81e8b3f7d015a8c80";
+ return "256f4673854c0f4399d4768c7df73922";
#elif BUILDFLAG(IS_APPLE)
- return "fab7e76a223f7fd4f2f9da88b2ced171";
+ return "57a55882da9caf4d0feb2de68fac2f94";
#else
- return "5efd7ff61ff87e5a615a546c55450c7d";
+ return "92d2c4d7d8e56edd972f7f092c815bb0";
#endif
}
#if BUILDFLAG(IS_APPLE)
diff --git a/fpdfsdk/fpdf_view.cpp b/fpdfsdk/fpdf_view.cpp
index 2c9c79c..f978330 100644
--- a/fpdfsdk/fpdf_view.cpp
+++ b/fpdfsdk/fpdf_view.cpp
@@ -967,12 +967,50 @@
}
CHECK(!pBitmap->IsPremultiplied());
- CFX_DefaultRenderDevice device;
- device.Attach(pBitmap);
- if (!pBitmap->IsAlphaFormat())
+ FX_RECT fill_rect(left, top, left + width, top + height);
+
+ if (!pBitmap->IsAlphaFormat()) {
color |= 0xFF000000;
- device.FillRect(FX_RECT(left, top, left + width, top + height),
- static_cast<uint32_t>(color));
+ }
+
+ // Let CFX_DefaultRenderDevice handle the 8-bit case.
+ const int bpp = pBitmap->GetBPP();
+ if (bpp == 8) {
+ CFX_DefaultRenderDevice device;
+ device.Attach(std::move(pBitmap));
+ device.FillRect(fill_rect, static_cast<uint32_t>(color));
+ return;
+ }
+
+ // Handle filling 24/32-bit bitmaps directly without CFX_DefaultRenderDevice.
+ // When CFX_DefaultRenderDevice is using Skia, this avoids extra work to
+ // change `pBitmap` to be premultiplied and back, or extra work to change
+ // `pBitmap` to 32 BPP and back.
+ fill_rect.Intersect(FX_RECT(0, 0, pBitmap->GetWidth(), pBitmap->GetHeight()));
+ if (fill_rect.IsEmpty()) {
+ return;
+ }
+
+ const int row_end = fill_rect.top + fill_rect.Height();
+ if (bpp == 32) {
+ for (int row = fill_rect.top; row < row_end; ++row) {
+ auto span32 = pBitmap->GetWritableScanlineAs<uint32_t>(row).subspan(
+ fill_rect.left, fill_rect.Width());
+ fxcrt::Fill(span32, static_cast<uint32_t>(color));
+ }
+ return;
+ }
+
+ CHECK_EQ(bpp, 24);
+ const FX_BGR_STRUCT<uint8_t> bgr = {.blue = FXARGB_B(color),
+ .green = FXARGB_G(color),
+ .red = FXARGB_R(color)};
+ for (int row = fill_rect.top; row < row_end; ++row) {
+ auto bgr_span =
+ pBitmap->GetWritableScanlineAs<FX_BGR_STRUCT<uint8_t>>(row).subspan(
+ fill_rect.left, fill_rect.Width());
+ fxcrt::Fill(bgr_span, bgr);
+ }
}
FPDF_EXPORT void* FPDF_CALLCONV FPDFBitmap_GetBuffer(FPDF_BITMAP bitmap) {
