Add initial tests for SetClip_PathFill()
Debugging for imaging issue in [1] revealed unexpected bad behavior
coming out of Windows GDI API. Microsoft documentation [2] indicates
that if a bad clip is provided (e.g., cause an error with
IntersectClipRect()) then the current clipping region should be
unaffected. The disappearance of images has shown that this isn't
necessarily true in all cases, and is something to be avoided.
Update CGdiDeviceDriver::SetClip_PathFill() to watch for an error
condition from IntersectClipRect() and return failure if it happens.
Add some new tests for this including to illustrate triggering this
GDI condition.
[1] https://crbug.com/1019026
[2] https://docs.microsoft.com/en-us/windows/win32/api/wingdi/nf-wingdi-intersectcliprect
Bug: chromium:1019026
Change-Id: I336617399bf90b511de5d6fbecf3da829fbe4de6
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/62470
Commit-Queue: Alan Screen <awscreen@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/core/fxge/BUILD.gn b/core/fxge/BUILD.gn
index b2a1a1c..ad47e0a 100644
--- a/core/fxge/BUILD.gn
+++ b/core/fxge/BUILD.gn
@@ -228,4 +228,12 @@
"//skia",
]
}
+
+ if (is_win) {
+ sources += [ "win32/fx_win32_device_embeddertest.cpp" ]
+ deps += [
+ ":fxge",
+ "../fxcodec",
+ ]
+ }
}
diff --git a/core/fxge/win32/fx_win32_device.cpp b/core/fxge/win32/fx_win32_device.cpp
index 4f20d68..80b9d36 100644
--- a/core/fxge/win32/fx_win32_device.cpp
+++ b/core/fxge/win32/fx_win32_device.cpp
@@ -1085,8 +1085,8 @@
CFX_FloatRect rectf;
if (pPathData->IsRect(pMatrix, &rectf)) {
FX_RECT rect = rectf.GetOuterRect();
- IntersectClipRect(m_hDC, rect.left, rect.top, rect.right, rect.bottom);
- return true;
+ return IntersectClipRect(m_hDC, rect.left, rect.top, rect.right,
+ rect.bottom) != ERROR;
}
}
SetPathToDC(m_hDC, pPathData, pMatrix);
diff --git a/core/fxge/win32/fx_win32_device_embeddertest.cpp b/core/fxge/win32/fx_win32_device_embeddertest.cpp
new file mode 100644
index 0000000..91daa20
--- /dev/null
+++ b/core/fxge/win32/fx_win32_device_embeddertest.cpp
@@ -0,0 +1,77 @@
+// Copyright 2019 PDFium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "core/fxge/win32/win32_int.h"
+
+#include <windows.h>
+
+#include <memory>
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+constexpr CFX_Matrix kIdentityMatrix;
+
+} // namespace
+
+class CFX_WindowsRenderDeviceTest : public testing::Test {
+ public:
+ void SetUp() override {
+ // Get a device context with Windows GDI.
+ m_hDC = CreateCompatibleDC(nullptr);
+ ASSERT_TRUE(m_hDC);
+ CFX_GEModule::Create(nullptr);
+ m_driver = std::make_unique<CFX_WindowsRenderDevice>(m_hDC, nullptr);
+ m_driver->SaveState();
+ }
+
+ void TearDown() override {
+ m_driver->RestoreState(false);
+ m_driver.reset();
+ CFX_GEModule::Destroy();
+ DeleteDC(m_hDC);
+ }
+
+ protected:
+ HDC m_hDC;
+ std::unique_ptr<CFX_WindowsRenderDevice> m_driver;
+};
+
+TEST_F(CFX_WindowsRenderDeviceTest, SimpleClipTriangle) {
+ CFX_PathData path_data;
+ CFX_PointF p1(0.0f, 0.0f);
+ CFX_PointF p2(0.0f, 100.0f);
+ CFX_PointF p3(100.0f, 100.0f);
+
+ path_data.AppendLine(p1, p2);
+ path_data.AppendLine(p2, p3);
+ path_data.AppendLine(p3, p1);
+ path_data.ClosePath();
+ EXPECT_TRUE(
+ m_driver->SetClip_PathFill(&path_data, &kIdentityMatrix, FXFILL_WINDING));
+}
+
+TEST_F(CFX_WindowsRenderDeviceTest, SimpleClipRect) {
+ CFX_PathData path_data;
+
+ path_data.AppendRect(0.0f, 100.0f, 200.0f, 0.0f);
+ path_data.ClosePath();
+ EXPECT_TRUE(
+ m_driver->SetClip_PathFill(&path_data, &kIdentityMatrix, FXFILL_WINDING));
+}
+
+TEST_F(CFX_WindowsRenderDeviceTest, GargantuanClipRect) {
+ CFX_PathData path_data;
+
+ path_data.AppendRect(-257698020.0f, -257697252.0f, 257698044.0f,
+ 257698812.0f);
+ path_data.ClosePath();
+ // These coordinates for a clip path are valid, just very large. Using these
+ // for a clip path should allow IntersectClipRect() to return success;
+ // however they do not because the GDI API IntersectClipRect() errors out and
+ // affect subsequent imaging. crbug.com/1019026
+ EXPECT_FALSE(
+ m_driver->SetClip_PathFill(&path_data, &kIdentityMatrix, FXFILL_WINDING));
+}
