Implement CFX_SkiaDeviceDriver::GetClipBox()
Correctly implements CFX_SkiaDeviceDriver::GetClipBox() using SkCanvas's
getDeviceClipBounds() API, as per the TODO. The old implementation
always used the entire device as the clip box, leading to extreme
overdraw for tiled patterns in particular.
Fixed: pdfium:2024
Change-Id: I5cc1ff73991d86edf0036c8c67b767bc5103ce3c
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/106330
Commit-Queue: K. Moon <kmoon@chromium.org>
Reviewed-by: Nigi <nigi@chromium.org>
diff --git a/DEPS b/DEPS
index d569281..e5aa4d8 100644
--- a/DEPS
+++ b/DEPS
@@ -140,7 +140,7 @@
# Three lines of non-changing comments so that
# the commit queue can handle CLs rolling pdfium_tests
# and whatever else without interference from each other.
- 'pdfium_tests_revision': 'eb4062246a81698801fbfcd4a9b68328ab2d8895',
+ 'pdfium_tests_revision': 'd1386521f5d606b9110143aa40b23651b17aded2',
# Three lines of non-changing comments so that
# the commit queue can handle CLs rolling resultdb
# and whatever else without interference from each other.
diff --git a/core/fxge/BUILD.gn b/core/fxge/BUILD.gn
index 307b5b1..c526c06 100644
--- a/core/fxge/BUILD.gn
+++ b/core/fxge/BUILD.gn
@@ -204,6 +204,7 @@
pdfium_unittest_source_set("unittests") {
sources = [
+ "cfx_defaultrenderdevice_unittest.cpp",
"cfx_folderfontinfo_unittest.cpp",
"cfx_fontmapper_unittest.cpp",
"cfx_path_unittest.cpp",
diff --git a/core/fxge/cfx_defaultrenderdevice_unittest.cpp b/core/fxge/cfx_defaultrenderdevice_unittest.cpp
new file mode 100644
index 0000000..9304a8c
--- /dev/null
+++ b/core/fxge/cfx_defaultrenderdevice_unittest.cpp
@@ -0,0 +1,80 @@
+// Copyright 2023 The PDFium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "core/fxge/cfx_defaultrenderdevice.h"
+
+#include "core/fxcrt/fx_coordinates.h"
+#include "core/fxcrt/retain_ptr.h"
+#include "core/fxge/cfx_fillrenderoptions.h"
+#include "core/fxge/cfx_graphstatedata.h"
+#include "core/fxge/cfx_path.h"
+#include "core/fxge/dib/cfx_dibitmap.h"
+#include "core/fxge/dib/fx_dib.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+TEST(CFX_DefaultRenderDeviceTest, GetClipBox_Default) {
+ CFX_DefaultRenderDevice device;
+ ASSERT_TRUE(device.Create(/*width=*/16, /*height=*/16, FXDIB_Format::kArgb,
+ /*pBackdropBitmap=*/nullptr));
+
+ EXPECT_EQ(FX_RECT(0, 0, 16, 16), device.GetClipBox());
+}
+
+TEST(CFX_DefaultRenderDeviceTest, GetClipBox_PathFill) {
+ // Matrix that transposes and translates by 1 unit on each axis.
+ const CFX_Matrix object_to_device(0, 1, 1, 0, 1, -1);
+
+ // Fill type cannot be none.
+ const CFX_FillRenderOptions fill_options(
+ CFX_FillRenderOptions::FillType::kEvenOdd);
+
+ CFX_DefaultRenderDevice device;
+ ASSERT_TRUE(device.Create(/*width=*/16, /*height=*/16, FXDIB_Format::kArgb,
+ /*pBackdropBitmap=*/nullptr));
+
+ CFX_Path path;
+ path.AppendRect(2, 4, 14, 12);
+ EXPECT_TRUE(device.SetClip_PathFill(path, &object_to_device, fill_options));
+
+ EXPECT_EQ(FX_RECT(5, 1, 13, 13), device.GetClipBox());
+}
+
+TEST(CFX_DefaultRenderDeviceTest, GetClipBox_PathStroke) {
+ // Matrix that transposes and translates by 1 unit on each axis.
+ const CFX_Matrix object_to_device(0, 1, 1, 0, 1, -1);
+
+ // Default line width is 1.
+ const CFX_GraphStateData graphics_state;
+
+ CFX_DefaultRenderDevice device;
+ ASSERT_TRUE(device.Create(/*width=*/16, /*height=*/16, FXDIB_Format::kArgb,
+ /*pBackdropBitmap=*/nullptr));
+
+ CFX_Path path;
+ path.AppendRect(2, 4, 14, 12);
+ EXPECT_TRUE(
+ device.SetClip_PathStroke(path, &object_to_device, &graphics_state));
+
+ EXPECT_EQ(FX_RECT(4, 0, 14, 14), device.GetClipBox());
+}
+
+TEST(CFX_DefaultRenderDeviceTest, GetClipBox_Rect) {
+ CFX_DefaultRenderDevice device;
+ ASSERT_TRUE(device.Create(/*width=*/16, /*height=*/16, FXDIB_Format::kArgb,
+ /*pBackdropBitmap=*/nullptr));
+
+ EXPECT_TRUE(device.SetClip_Rect({2, 4, 14, 12}));
+
+ EXPECT_EQ(FX_RECT(2, 4, 14, 12), device.GetClipBox());
+}
+
+TEST(CFX_DefaultRenderDeviceTest, GetClipBox_Empty) {
+ CFX_DefaultRenderDevice device;
+ ASSERT_TRUE(device.Create(/*width=*/16, /*height=*/16, FXDIB_Format::kArgb,
+ /*pBackdropBitmap=*/nullptr));
+
+ EXPECT_TRUE(device.SetClip_Rect({2, 8, 14, 8}));
+
+ EXPECT_TRUE(device.GetClipBox().IsEmpty());
+}
diff --git a/core/fxge/skia/fx_skia_device.cpp b/core/fxge/skia/fx_skia_device.cpp
index 09a73dd..ef3218d 100644
--- a/core/fxge/skia/fx_skia_device.cpp
+++ b/core/fxge/skia/fx_skia_device.cpp
@@ -1504,12 +1504,11 @@
}
bool CFX_SkiaDeviceDriver::GetClipBox(FX_RECT* pRect) {
- // TODO(caryclark) call m_canvas->getClipDeviceBounds() instead
- pRect->left = 0;
- pRect->top = 0;
- const SkImageInfo& canvasSize = m_pCanvas->imageInfo();
- pRect->right = canvasSize.width();
- pRect->bottom = canvasSize.height();
+ SkIRect clip = m_pCanvas->getDeviceClipBounds();
+ pRect->left = clip.fLeft;
+ pRect->top = clip.fTop;
+ pRect->right = clip.fRight;
+ pRect->bottom = clip.fBottom;
return true;
}
diff --git a/testing/resources/pixel/bug_1258968_expected_skia.pdf.0.png b/testing/resources/pixel/bug_1258968_expected_skia.pdf.0.png
index 985259c..18f4279 100644
--- a/testing/resources/pixel/bug_1258968_expected_skia.pdf.0.png
+++ b/testing/resources/pixel/bug_1258968_expected_skia.pdf.0.png
Binary files differ
diff --git a/testing/resources/pixel/bug_1288_2_expected_skia.pdf.0.png b/testing/resources/pixel/bug_1288_2_expected_skia.pdf.0.png
index bac4bb1..3a8cbee 100644
--- a/testing/resources/pixel/bug_1288_2_expected_skia.pdf.0.png
+++ b/testing/resources/pixel/bug_1288_2_expected_skia.pdf.0.png
Binary files differ
