From 82bac7e665840448e153165d2e5f316f4bdc07ae Mon Sep 17 00:00:00 2001
From: Rodrigo Tobar <rtobarc@gmail.com>
Date: Fri, 3 Feb 2023 07:11:27 +0800
Subject: [PATCH] LibPDF: Fix clipping of painting operations

While the clipping logic was correct (current v/s new clipping path),
the clipping path contents weren't. This commit fixed that.

We calculate the clipping path in two places: when we set it to be the
whole page at graphics state creation time, and when we perform clipping
path intersection to calculate a new clipping path. The clipping path is
then used to limit painting by passing it to the painter (more
precisely, but passing its bounding box to the painter, as the latter
doesn't support arbitrary path clipping). For this last point the
clipping path must be in device coordinates.

There was however a mix of coordinate systems involved in the creation,
update and usage of the clipping path:

 * The initial values of the path (i.e., the whole page) were in user
   coordinates.
 * Clipping path intersection was performed against m_current_path,
   which is in device coordinates.
 * To perform the clipping operation, the current clipping path was
   assumed to be in user coordinates.

This mix resulted in the clipping not working correctly depending on the
zoom level at which one visualised a page.

This commit fixes the issue by always keeping track of the clipping path
in device coordinates. This means that the initial full-page contents
are now converted to device coordinates before putting them in the
graphics state, and that no mapping is performed when applied the
clipping to the painter.
---
 Userland/Libraries/LibPDF/Renderer.cpp | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/Userland/Libraries/LibPDF/Renderer.cpp b/Userland/Libraries/LibPDF/Renderer.cpp
index 47fd771b4d..3c31ed6b03 100644
--- a/Userland/Libraries/LibPDF/Renderer.cpp
+++ b/Userland/Libraries/LibPDF/Renderer.cpp
@@ -35,13 +35,6 @@ static void rect_path(Gfx::Path& path, float x, float y, float width, float heig
     path.close();
 }
 
-static Gfx::Path rect_path(float x, float y, float width, float height)
-{
-    Gfx::Path path;
-    rect_path(path, x, y, width, height);
-    return path;
-}
-
 template<typename T>
 static void rect_path(Gfx::Path& path, Gfx::Rect<T> rect)
 {
@@ -49,7 +42,7 @@ static void rect_path(Gfx::Path& path, Gfx::Rect<T> rect)
 }
 
 template<typename T>
-static Gfx::Path rect_path(Gfx::Rect<T> rect)
+static Gfx::Path rect_path(Gfx::Rect<T> const& rect)
 {
     Gfx::Path path;
     rect_path(path, rect);
@@ -84,7 +77,7 @@ Renderer::Renderer(RefPtr<Document> document, Page const& page, RefPtr<Gfx::Bitm
     userspace_matrix.multiply(horizontal_reflection_matrix);
     userspace_matrix.translate(0.0f, -height);
 
-    auto initial_clipping_path = rect_path(0, 0, width, height);
+    auto initial_clipping_path = rect_path(userspace_matrix.map(Gfx::FloatRect(0, 0, width, height)));
     m_graphics_state_stack.append(GraphicsState { userspace_matrix, { initial_clipping_path, initial_clipping_path } });
 
     m_bitmap->fill(Gfx::Color::NamedColor::White);
@@ -285,7 +278,7 @@ RENDERER_HANDLER(path_append_rect)
 
 void Renderer::begin_path_paint()
 {
-    auto bounding_box = map(state().clipping_paths.current.bounding_box());
+    auto bounding_box = state().clipping_paths.current.bounding_box();
     m_painter.clear_clip_rect();
     if (m_rendering_preferences.show_clipping_paths) {
         m_painter.stroke_path(rect_path(bounding_box), Color::Black, 1);