Use PDF page base clip for Win32 rendering
Introduce a base clip for a PDF page. This base clip is the same as the
initial clip rectangle that is created when rendering for a page is
started; it is now also remembered for later use.
Utilize this base clip for rendering with the Win32 device. This
provides extra protection in some cases against otherwise valid clip
paths for which the coordinates are so large that they trigger errors
when used with the Windows GDI API.
It is not easy to apply this in all cases since some paths could be
impacted by clipping too early (e.g., if applying it affected fill
mode for complicated paths), and such paths are provided straight to
GDI. Start with an initial easy fix of applying this clip against
rectangular paths, which resolves an issue seen from at least one PDF
generated by Microsoft Word that had a rectangular clip extending
hundreds of millions of pixels beyond the page boundaries, having no
effective use yet was triggering coordinate issues and causing images to
be erroneously discarded.
Bug: chromium:1019026
Change-Id: I2c5e6e1cd6444b8d4357bd812453d89eabf1ab81
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/62353
Commit-Queue: Alan Screen <awscreen@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/core/fxge/cfx_renderdevice.cpp b/core/fxge/cfx_renderdevice.cpp
index fdfdd6a..b1474c5 100644
--- a/core/fxge/cfx_renderdevice.cpp
+++ b/core/fxge/cfx_renderdevice.cpp
@@ -458,6 +458,10 @@
m_RenderCaps & FXRC_ALPHA_OUTPUT ? FXDIB_Argb : kPlatformFormat);
}
+void CFX_RenderDevice::SetBaseClip(const FX_RECT& rect) {
+ m_pDeviceDriver->SetBaseClip(rect);
+}
+
bool CFX_RenderDevice::SetClip_PathFill(const CFX_PathData* pPathData,
const CFX_Matrix* pObject2Device,
int fill_mode) {
diff --git a/core/fxge/cfx_renderdevice.h b/core/fxge/cfx_renderdevice.h
index b8d9221..755f445 100644
--- a/core/fxge/cfx_renderdevice.h
+++ b/core/fxge/cfx_renderdevice.h
@@ -67,13 +67,14 @@
int width,
int height) const;
const FX_RECT& GetClipBox() const { return m_ClipBox; }
- bool SetClip_Rect(const FX_RECT& pRect);
+ void SetBaseClip(const FX_RECT& rect);
bool SetClip_PathFill(const CFX_PathData* pPathData,
const CFX_Matrix* pObject2Device,
int fill_mode);
bool SetClip_PathStroke(const CFX_PathData* pPathData,
const CFX_Matrix* pObject2Device,
const CFX_GraphStateData* pGraphState);
+ bool SetClip_Rect(const FX_RECT& pRect);
bool DrawPath(const CFX_PathData* pPathData,
const CFX_Matrix* pObject2Device,
const CFX_GraphStateData* pGraphState,
diff --git a/core/fxge/renderdevicedriver_iface.cpp b/core/fxge/renderdevicedriver_iface.cpp
index acb81d6..f8fcccf 100644
--- a/core/fxge/renderdevicedriver_iface.cpp
+++ b/core/fxge/renderdevicedriver_iface.cpp
@@ -25,6 +25,8 @@
return false;
}
+void RenderDeviceDriverIface::SetBaseClip(const FX_RECT& rect) {}
+
bool RenderDeviceDriverIface::SetPixel(int x, int y, uint32_t color) {
return false;
}
diff --git a/core/fxge/renderdevicedriver_iface.h b/core/fxge/renderdevicedriver_iface.h
index 7524f90..69e404b 100644
--- a/core/fxge/renderdevicedriver_iface.h
+++ b/core/fxge/renderdevicedriver_iface.h
@@ -44,6 +44,7 @@
virtual void SaveState() = 0;
virtual void RestoreState(bool bKeepSaved) = 0;
+ virtual void SetBaseClip(const FX_RECT& rect);
virtual bool SetClip_PathFill(const CFX_PathData* pPathData,
const CFX_Matrix* pObject2Device,
int fill_mode) = 0;
diff --git a/core/fxge/win32/fx_win32_device.cpp b/core/fxge/win32/fx_win32_device.cpp
index 80b9d36..bb13a76 100644
--- a/core/fxge/win32/fx_win32_device.cpp
+++ b/core/fxge/win32/fx_win32_device.cpp
@@ -1078,6 +1078,10 @@
return true;
}
+void CGdiDeviceDriver::SetBaseClip(const FX_RECT& rect) {
+ m_BaseClipBox = rect;
+}
+
bool CGdiDeviceDriver::SetClip_PathFill(const CFX_PathData* pPathData,
const CFX_Matrix* pMatrix,
int fill_mode) {
@@ -1085,6 +1089,10 @@
CFX_FloatRect rectf;
if (pPathData->IsRect(pMatrix, &rectf)) {
FX_RECT rect = rectf.GetOuterRect();
+ // Can easily apply base clip to protect against wildly large rectangular
+ // clips. crbug.com/1019026
+ if (m_BaseClipBox.has_value())
+ rect.Intersect(m_BaseClipBox.value());
return IntersectClipRect(m_hDC, rect.left, rect.top, rect.right,
rect.bottom) != ERROR;
}
diff --git a/core/fxge/win32/fx_win32_device_embeddertest.cpp b/core/fxge/win32/fx_win32_device_embeddertest.cpp
index 91daa20..cb088e7 100644
--- a/core/fxge/win32/fx_win32_device_embeddertest.cpp
+++ b/core/fxge/win32/fx_win32_device_embeddertest.cpp
@@ -9,6 +9,7 @@
#include <memory>
#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/base/ptr_util.h"
namespace {
@@ -23,7 +24,7 @@
m_hDC = CreateCompatibleDC(nullptr);
ASSERT_TRUE(m_hDC);
CFX_GEModule::Create(nullptr);
- m_driver = std::make_unique<CFX_WindowsRenderDevice>(m_hDC, nullptr);
+ m_driver = pdfium::MakeUnique<CFX_WindowsRenderDevice>(m_hDC, nullptr);
m_driver->SaveState();
}
@@ -75,3 +76,17 @@
EXPECT_FALSE(
m_driver->SetClip_PathFill(&path_data, &kIdentityMatrix, FXFILL_WINDING));
}
+
+TEST_F(CFX_WindowsRenderDeviceTest, GargantuanClipRectWithBaseClip) {
+ CFX_PathData path_data;
+ const FX_RECT kBaseClip(0, 0, 5100, 6600);
+
+ m_driver->SetBaseClip(kBaseClip);
+ path_data.AppendRect(-257698020.0f, -257697252.0f, 257698044.0f,
+ 257698812.0f);
+ path_data.ClosePath();
+ // Use of a reasonable base clip ensures that we avoid getting an error back
+ // from GDI API IntersectClipRect().
+ EXPECT_TRUE(
+ m_driver->SetClip_PathFill(&path_data, &kIdentityMatrix, FXFILL_WINDING));
+}
diff --git a/core/fxge/win32/win32_int.h b/core/fxge/win32/win32_int.h
index 77d746e..7f299ef 100644
--- a/core/fxge/win32/win32_int.h
+++ b/core/fxge/win32/win32_int.h
@@ -19,6 +19,7 @@
#include "core/fxge/renderdevicedriver_iface.h"
#include "core/fxge/win32/cfx_psrenderer.h"
#include "core/fxge/win32/cpsoutput.h"
+#include "third_party/base/optional.h"
class CFX_ImageRenderer;
class TextCharPos;
@@ -81,6 +82,7 @@
int GetDeviceCaps(int caps_id) const override;
void SaveState() override;
void RestoreState(bool bKeepSaved) override;
+ void SetBaseClip(const FX_RECT& rect) override;
bool SetClip_PathFill(const CFX_PathData* pPathData,
const CFX_Matrix* pObject2Device,
int fill_mode) override;
@@ -129,6 +131,7 @@
int m_nBitsPerPixel;
const DeviceType m_DeviceType;
int m_RenderCaps;
+ pdfium::Optional<FX_RECT> m_BaseClipBox;
};
class CGdiDisplayDriver final : public CGdiDeviceDriver {
diff --git a/fpdfsdk/fpdf_view.cpp b/fpdfsdk/fpdf_view.cpp
index a735554..a4a70f4 100644
--- a/fpdfsdk/fpdf_view.cpp
+++ b/fpdfsdk/fpdf_view.cpp
@@ -107,6 +107,7 @@
pdfium::MakeRetain<CPDF_OCContext>(pPage->GetDocument(), usage));
pContext->m_pDevice->SaveState();
+ pContext->m_pDevice->SetBaseClip(clipping_rect);
pContext->m_pDevice->SetClip_Rect(clipping_rect);
pContext->m_pContext = pdfium::MakeUnique<CPDF_RenderContext>(pPage);
pContext->m_pContext->AppendLayer(pPage, &matrix);
