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)); +}