[Skia] Fix issues for printing 24 BPP images When using `CFX_SkiaDeviceDriver` for rendering to a 24 BPP bitmap, it creates an internal bitmap buffer for fast processing and saves the actual output bitmap as `m_pOriginalBitmap`. Keeping both the original and the internal bitmaps up-to-date is the key to get the correct printing result. This CL improves the rendering process in the following ways: 1. For `CFX_SkiaDeviceDriver` only: Add `CFX_RenderDevice::SyncInternalBitmaps()` method to make the underlying `m_pDeviceDriver` transfer the rendering result from its internal buffer to the `CFX_RenderDevice` bitmap. This allows syncing without destroying the `m_pDeviceDriver`. 2. In `CPDF_DeviceBuffer::OutputToDevice()`, `pBuffer` is used for creating a `CFX_RenderDevice` already before it got passed into `GetBackground()` to be painted white. This means the "old" `CFX_RenderDevice`'s device driver contains an internal buffer that's out-dated (without the white paint). To keep the existing `CFX_RenderDevice` buffers fresh, use it as a parameter of `GetBackground()` and replace `pBuffer` so that its buffer gets updated and we can avoid creating a new layer of `CFX_RenderDevice`. Also, removes the pixel tests that are fixed by this CL from the suppression list. Bug: chromium:1470768, pdfium:2054 Change-Id: I8442664ae58dc528442b40a8503100f9501f5ea1 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/111431 Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: Nigi <nigi@chromium.org>
diff --git a/core/fpdfapi/render/cpdf_devicebuffer.cpp b/core/fpdfapi/render/cpdf_devicebuffer.cpp index b7722f1..5bf308a 100644 --- a/core/fpdfapi/render/cpdf_devicebuffer.cpp +++ b/core/fpdfapi/render/cpdf_devicebuffer.cpp
@@ -84,7 +84,7 @@ auto pBuffer = pdfium::MakeRetain<CFX_DIBitmap>(); m_pDevice->CreateCompatibleBitmap(pBuffer, m_pBitmap->GetWidth(), m_pBitmap->GetHeight()); - m_pContext->GetBackground(pBuffer, m_pObject, nullptr, m_Matrix); + m_pContext->GetBackground(m_pDevice, m_pObject, nullptr, m_Matrix); pBuffer->CompositeBitmap(0, 0, pBuffer->GetWidth(), pBuffer->GetHeight(), m_pBitmap, 0, 0, BlendMode::kNormal, nullptr, false); m_pDevice->StretchDIBits(pBuffer, m_Rect.left, m_Rect.top, m_Rect.Width(),
diff --git a/core/fpdfapi/render/cpdf_rendercontext.cpp b/core/fpdfapi/render/cpdf_rendercontext.cpp index 4116840..7589b41 100644 --- a/core/fpdfapi/render/cpdf_rendercontext.cpp +++ b/core/fpdfapi/render/cpdf_rendercontext.cpp
@@ -21,6 +21,7 @@ #include "core/fxge/cfx_renderdevice.h" #include "core/fxge/dib/cfx_dibitmap.h" #include "core/fxge/dib/fx_dib.h" +#include "third_party/base/check.h" CPDF_RenderContext::CPDF_RenderContext( CPDF_Document* pDoc, @@ -32,15 +33,14 @@ CPDF_RenderContext::~CPDF_RenderContext() = default; -void CPDF_RenderContext::GetBackground(RetainPtr<CFX_DIBitmap> pBuffer, +void CPDF_RenderContext::GetBackground(CFX_RenderDevice* pDevice, const CPDF_PageObject* pObj, const CPDF_RenderOptions* pOptions, const CFX_Matrix& mtFinal) { - CFX_DefaultRenderDevice device; - device.Attach(std::move(pBuffer)); - device.FillRect(FX_RECT(0, 0, device.GetWidth(), device.GetHeight()), - 0xffffffff); - Render(&device, pObj, pOptions, &mtFinal); + CHECK(pDevice); + pDevice->FillRect(FX_RECT(0, 0, pDevice->GetWidth(), pDevice->GetHeight()), + 0xffffffff); + Render(pDevice, pObj, pOptions, &mtFinal); } void CPDF_RenderContext::AppendLayer(CPDF_PageObjectHolder* pObjectHolder,
diff --git a/core/fpdfapi/render/cpdf_rendercontext.h b/core/fpdfapi/render/cpdf_rendercontext.h index 383e798..1012e81 100644 --- a/core/fpdfapi/render/cpdf_rendercontext.h +++ b/core/fpdfapi/render/cpdf_rendercontext.h
@@ -13,7 +13,6 @@ #include "core/fxcrt/retain_ptr.h" #include "core/fxcrt/unowned_ptr.h" -class CFX_DIBitmap; class CFX_Matrix; class CFX_RenderDevice; class CPDF_Dictionary; @@ -52,7 +51,7 @@ const CPDF_RenderOptions* pOptions, const CFX_Matrix* pLastMatrix); - void GetBackground(RetainPtr<CFX_DIBitmap> pBuffer, + void GetBackground(CFX_RenderDevice* pDevice, const CPDF_PageObject* pObj, const CPDF_RenderOptions* pOptions, const CFX_Matrix& mtFinal);
diff --git a/core/fpdfapi/render/cpdf_scaledrenderbuffer.cpp b/core/fpdfapi/render/cpdf_scaledrenderbuffer.cpp index 26e8b7f..4cba877 100644 --- a/core/fpdfapi/render/cpdf_scaledrenderbuffer.cpp +++ b/core/fpdfapi/render/cpdf_scaledrenderbuffer.cpp
@@ -57,8 +57,7 @@ } m_Matrix.Scale(0.5f, 0.5f); } - pContext->GetBackground(m_pBitmapDevice->GetBitmap(), pObj, pOptions, - m_Matrix); + pContext->GetBackground(m_pBitmapDevice.get(), pObj, pOptions, m_Matrix); return true; } @@ -69,6 +68,11 @@ void CPDF_ScaledRenderBuffer::OutputToDevice() { if (m_pBitmapDevice) { +#if defined(_SKIA_SUPPORT_) + if (!m_pBitmapDevice->SyncInternalBitmaps()) { + return; + } +#endif m_pDevice->StretchDIBits(m_pBitmapDevice->GetBitmap(), m_Rect.left, m_Rect.top, m_Rect.Width(), m_Rect.Height()); }
diff --git a/core/fxge/cfx_renderdevice.cpp b/core/fxge/cfx_renderdevice.cpp index 2280f6f..93c3306 100644 --- a/core/fxge/cfx_renderdevice.cpp +++ b/core/fxge/cfx_renderdevice.cpp
@@ -1002,6 +1002,10 @@ return m_pDeviceDriver->SetBitsWithMask(pBitmap, pMask, left, top, bitmap_alpha, blend_type); } + +bool CFX_RenderDevice::SyncInternalBitmaps() { + return m_pDeviceDriver->SyncInternalBitmaps(); +} #endif bool CFX_RenderDevice::DrawNormalText(pdfium::span<const TextCharPos> pCharPos,
diff --git a/core/fxge/cfx_renderdevice.h b/core/fxge/cfx_renderdevice.h index 8e6e380..b7ce809 100644 --- a/core/fxge/cfx_renderdevice.h +++ b/core/fxge/cfx_renderdevice.h
@@ -221,6 +221,7 @@ int top, int bitmap_alpha, BlendMode blend_type); + bool SyncInternalBitmaps(); #endif protected:
diff --git a/core/fxge/renderdevicedriver_iface.cpp b/core/fxge/renderdevicedriver_iface.cpp index b549ea2..1b97a08 100644 --- a/core/fxge/renderdevicedriver_iface.cpp +++ b/core/fxge/renderdevicedriver_iface.cpp
@@ -83,4 +83,8 @@ } void RenderDeviceDriverIface::SetGroupKnockout(bool group_knockout) {} + +bool RenderDeviceDriverIface::SyncInternalBitmaps() { + return true; +} #endif
diff --git a/core/fxge/renderdevicedriver_iface.h b/core/fxge/renderdevicedriver_iface.h index 09dd93c..4f9690b 100644 --- a/core/fxge/renderdevicedriver_iface.h +++ b/core/fxge/renderdevicedriver_iface.h
@@ -116,6 +116,11 @@ int bitmap_alpha, BlendMode blend_type); virtual void SetGroupKnockout(bool group_knockout); + + // For `CFX_SkiaDeviceDriver` only: + // Syncs the current rendering result from the internal buffer to the output + // bitmap if such internal buffer exists. + virtual bool SyncInternalBitmaps(); #endif // Multiplies the device by a constant alpha, returning `true` on success.
diff --git a/core/fxge/skia/fx_skia_device.cpp b/core/fxge/skia/fx_skia_device.cpp index 25d4a1e..6a79272 100644 --- a/core/fxge/skia/fx_skia_device.cpp +++ b/core/fxge/skia/fx_skia_device.cpp
@@ -1541,6 +1541,22 @@ m_bGroupKnockout = group_knockout; } +bool CFX_SkiaDeviceDriver::SyncInternalBitmaps() { + if (!m_pOriginalBitmap) { + return true; + } + + 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); + return true; +} + void CFX_SkiaDeviceDriver::Clear(uint32_t color) { m_pCanvas->clear(color); }
diff --git a/core/fxge/skia/fx_skia_device.h b/core/fxge/skia/fx_skia_device.h index c66ceb8..29d699b 100644 --- a/core/fxge/skia/fx_skia_device.h +++ b/core/fxge/skia/fx_skia_device.h
@@ -108,6 +108,7 @@ int bitmap_alpha, BlendMode blend_type) override; void SetGroupKnockout(bool group_knockout) override; + bool SyncInternalBitmaps() override; bool StretchDIBits(const RetainPtr<CFX_DIBBase>& pBitmap, uint32_t color,
diff --git a/testing/SUPPRESSIONS b/testing/SUPPRESSIONS index 377aef5..51eefa4 100644 --- a/testing/SUPPRESSIONS +++ b/testing/SUPPRESSIONS
@@ -704,21 +704,12 @@ # GDI # TODO(pdfium:2054): Remove after associated bug is fixed -bug_1012369.in * * * gdi bug_1258968.in * * * gdi bug_1383708.in * * * gdi -bug_1469.in * * * gdi -bug_1693.in * * * gdi -bug_1733.in * * * gdi -bug_1750.in * * * gdi -bug_512557.pdf * * * gdi -bug_71.in * * * gdi bug_718762.in * * * gdi -bug_867501.pdf * * * gdi bug_966263.in * * * gdi bug_986108.in * * * gdi image_transformer_other.in * * * gdi -jpxdecode_without_smaskindata.in * * * gdi # TODO(pdfium:2055): Remove after associated bug is fixed barcode_test.pdf * * * gdi @@ -761,7 +752,6 @@ # TODO(pdfium:2056): Remove after associated bug is fixed bug_1015233.in * * * gdi -bug_1087.pdf * * * gdi bug_1099446.in * * * gdi bug_1161.in * * * gdi bug_1236.in * * * gdi @@ -771,13 +761,11 @@ bug_1396266.in * * * gdi bug_1430333.in * * * gdi bug_1822.in * * * gdi -bug_1845.in * * * gdi bug_1847.in * * * gdi bug_1949.in * * * gdi bug_1966.in * * * gdi bug_1972_1.in * * * gdi bug_1972_3.in * * * gdi -bug_1976.in * * * gdi bug_1995.in * * * gdi bug_451366.in * * * gdi bug_632.in * * * gdi