From 5eaa403ddf7aae1a2a6c0f5f844d716a946310ee Mon Sep 17 00:00:00 2001
From: Nico Weber <thakis@chromium.org>
Date: Thu, 16 Nov 2023 20:15:58 -0500
Subject: [PATCH] LibPDF: Use font dictionary object as cache key, not resource
 name

In the main page contents, /T0 might refer to a different font than
it might refer to in an XObject. So don't use the `Tf` argument as
font cache key. Instead, use the address of the font dictionary object.

Fixes false cache sharing, and also allows us to share cache entries
if the same font dict is referred to by two different names.

Fixes a regression from 2340e834cd (but keeps the speed-up intact).
---
 Userland/Libraries/LibPDF/Renderer.cpp | 16 ++++++++--------
 Userland/Libraries/LibPDF/Renderer.h   |  6 +++---
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/Userland/Libraries/LibPDF/Renderer.cpp b/Userland/Libraries/LibPDF/Renderer.cpp
index 3a309f4f00..37773924eb 100644
--- a/Userland/Libraries/LibPDF/Renderer.cpp
+++ b/Userland/Libraries/LibPDF/Renderer.cpp
@@ -406,17 +406,13 @@ RENDERER_HANDLER(text_set_leading)
     return {};
 }
 
-PDFErrorOr<NonnullRefPtr<PDFFont>> Renderer::get_font(FontCacheKey const& key, Optional<NonnullRefPtr<DictObject>> extra_resources)
+PDFErrorOr<NonnullRefPtr<PDFFont>> Renderer::get_font(FontCacheKey const& key)
 {
     auto it = m_font_cache.find(key);
     if (it != m_font_cache.end())
         return it->value;
 
-    auto resources = extra_resources.value_or(m_page.resources);
-    auto fonts_dictionary = MUST(resources->get_dict(m_document, CommonNames::Font));
-    auto font_dictionary = MUST(fonts_dictionary->get_dict(m_document, key.font_dictionary_key));
-
-    auto font = TRY(PDFFont::create(m_document, font_dictionary, key.font_size));
+    auto font = TRY(PDFFont::create(m_document, key.font_dictionary, key.font_size));
     m_font_cache.set(key, font);
     return font;
 }
@@ -430,8 +426,12 @@ RENDERER_HANDLER(text_set_font)
     auto& text_rendering_matrix = calculate_text_rendering_matrix();
     auto font_size = text_rendering_matrix.x_scale() * text_state().font_size;
 
-    FontCacheKey cache_key { target_font_name, font_size };
-    text_state().font = TRY(get_font(cache_key, extra_resources));
+    auto resources = extra_resources.value_or(m_page.resources);
+    auto fonts_dictionary = MUST(resources->get_dict(m_document, CommonNames::Font));
+    auto font_dictionary = MUST(fonts_dictionary->get_dict(m_document, target_font_name));
+
+    FontCacheKey cache_key { move(font_dictionary), font_size };
+    text_state().font = TRY(get_font(cache_key));
 
     m_text_rendering_matrix_is_dirty = true;
     return {};
diff --git a/Userland/Libraries/LibPDF/Renderer.h b/Userland/Libraries/LibPDF/Renderer.h
index 685e9194fa..21250a82f3 100644
--- a/Userland/Libraries/LibPDF/Renderer.h
+++ b/Userland/Libraries/LibPDF/Renderer.h
@@ -101,7 +101,7 @@ public:
     static PDFErrorsOr<void> render(Document&, Page const&, RefPtr<Gfx::Bitmap>, RenderingPreferences preferences);
 
     struct FontCacheKey {
-        DeprecatedString font_dictionary_key;
+        NonnullRefPtr<DictObject> font_dictionary;
         float font_size;
 
         bool operator==(FontCacheKey const&) const = default;
@@ -147,7 +147,7 @@ private:
     Gfx::AffineTransform const& calculate_text_rendering_matrix();
     Gfx::AffineTransform calculate_image_space_transformation(int width, int height);
 
-    PDFErrorOr<NonnullRefPtr<PDFFont>> get_font(FontCacheKey const&, Optional<NonnullRefPtr<DictObject>> extra_resources);
+    PDFErrorOr<NonnullRefPtr<PDFFont>> get_font(FontCacheKey const&);
 
     RefPtr<Document> m_document;
     RefPtr<Gfx::Bitmap> m_bitmap;
@@ -175,7 +175,7 @@ template<>
 struct Traits<PDF::Renderer::FontCacheKey> : public DefaultTraits<PDF::Renderer::FontCacheKey> {
     static unsigned hash(PDF::Renderer::FontCacheKey const& key)
     {
-        return pair_int_hash(key.font_dictionary_key.hash(), int_hash(bit_cast<u32>(key.font_size)));
+        return pair_int_hash(ptr_hash(key.font_dictionary.ptr()), int_hash(bit_cast<u32>(key.font_size)));
     }
 };