Tom Sepez (Gerrit)

unread,

Mar 12, 2026, 7:23:40 PM (5 days ago) Mar 12

to Lei Zhang, Pdfium LUCI CQ, pdfium-...@googlegroups.com

Attention needed from Lei Zhang

Tom Sepez voted Commit-Queue+1

Commit-Queue

+1

Open in Gerrit

Related details

Attention is currently required from:

Lei Zhang

Submit Requirements:

Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Lei Zhang (Gerrit)

unread,

Mar 12, 2026, 7:44:24 PM (5 days ago) Mar 12

to Tom Sepez, Lei Zhang, Pdfium LUCI CQ, pdfium-...@googlegroups.com

Attention needed from Tom Sepez

Lei Zhang added 4 comments

Commit Message

Line 13, Patchset 12 (Latest):-- Avoid runtime test for skia if not compiled into PDFium.

Lei Zhang . resolved

I think that was there to avoid writing so many ifdefs, but ok.

File core/fxge/cfx_gemodule.h

Line 74, Patchset 12 (Latest): const RendererType renderer_type_;

Lei Zhang . resolved

Maybe wrap in `#if defined(PDF_USE_SKIA)` since it's only read in UseSkiaRenderer()? Or just leave it, since mucking with conditional initializer lists is not fun.

File fpdfsdk/fpdf_view.cpp

Line 208, Patchset 12 (Latest): if (config->m_RendererType == FPDF_RENDERERTYPE_AGG ||

Lei Zhang . unresolved

CHECK() like before?

Line 263, Patchset 12 (Parent): // Note: we teardown/destroy things in reverse order.

Lei Zhang . unresolved

Keep the note.

Open in Gerrit

Related details

Attention is currently required from:

Tom Sepez

Submit Requirements:

Code-Owners
Code-Review

No-Unresolved-Comments

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Lei Zhang (Gerrit)

unread,

Mar 12, 2026, 7:44:30 PM (5 days ago) Mar 12

to Tom Sepez, Lei Zhang, Pdfium LUCI CQ, pdfium-...@googlegroups.com

Attention needed from Tom Sepez

Lei Zhang voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Tom Sepez

Submit Requirements:

Code-Owners
Code-Review

No-Unresolved-Comments

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Tom Sepez (Gerrit)

unread,

Mar 12, 2026, 7:54:52 PM (5 days ago) Mar 12

to Lei Zhang, Pdfium LUCI CQ, pdfium-...@googlegroups.com

Tom Sepez voted and added 3 comments

Votes added by Tom Sepez

Commit-Queue

+2

3 comments

File core/fxge/cfx_gemodule.h

Line 74, Patchset 12: const RendererType renderer_type_;

Lei Zhang . resolved

Maybe wrap in `#if defined(PDF_USE_SKIA)` since it's only read in UseSkiaRenderer()? Or just leave it, since mucking with conditional initializer lists is not fun.

Tom Sepez

Exactly. Makes calls ugly.

File fpdfsdk/fpdf_view.cpp

Line 208, Patchset 12: if (config->m_RendererType == FPDF_RENDERERTYPE_AGG ||

Lei Zhang . resolved

CHECK() like before?

Tom Sepez

Done

Line 263, Patchset 12 (Parent): // Note: we teardown/destroy things in reverse order.

Lei Zhang . resolved

Keep the note.

Tom Sepez

Done

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Tom Sepez (Gerrit)

unread,

Mar 12, 2026, 8:30:37 PM (5 days ago) Mar 12

to Lei Zhang, Pdfium LUCI CQ, pdfium-...@googlegroups.com

Tom Sepez voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Pdfium LUCI CQ (Gerrit)

unread,

Mar 12, 2026, 9:12:30 PM (5 days ago) Mar 12

to Tom Sepez, Lei Zhang, pdfium-...@googlegroups.com

Pdfium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

12 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: fpdfsdk/fpdf_view.cpp
Insertions: 26, Deletions: 14.

@@ -200,24 +200,35 @@
   FX_InitializeMemoryAllocators();
   CFX_Timer::InitializeGlobals();
 
-  CFX_FontMgr::FontBackend backend = CFX_FontMgr::FontBackend::kFreeType;
   CFX_GEModule::RendererType renderer_type =
       CFX_GEModule::RendererType::kDefault;
-#if defined(PDF_USE_SKIA)
   if (config && config->version >= 4) {
-    if (config->m_RendererType == FPDF_RENDERERTYPE_AGG ||
-        config->m_RendererType == FPDF_RENDERERTYPE_SKIA) {
-      renderer_type =
-          static_cast<CFX_GEModule::RendererType>(config->m_RendererType);
-    }
-  }
-  if (config && config->version >= 5 &&
-      config->m_FontLibraryType != FPDF_FONTBACKENDTYPE_FREETYPE) {
-    CHECK_EQ(config->m_FontLibraryType, FPDF_FONTBACKENDTYPE_FONTATIONS);
-    CHECK_EQ(config->m_RendererType, FPDF_RENDERERTYPE_SKIA);
-    backend = CFX_FontMgr::FontBackend::kFontations;
-  }
+#if defined(PDF_USE_SKIA)
+    CHECK(config->m_RendererType == FPDF_RENDERERTYPE_AGG ||
+          config->m_RendererType == FPDF_RENDERERTYPE_SKIA);
+    renderer_type =
+        static_cast<CFX_GEModule::RendererType>(config->m_RendererType);
+#else
+    // AGG-only builds should always use `FPDF_RENDERERTYPE_AGG`.
+    CHECK_EQ(config->m_RendererType, FPDF_RENDERERTYPE_AGG);
 #endif
+  }
+
+  CFX_FontMgr::FontBackend backend = CFX_FontMgr::FontBackend::kFreeType;
+  if (config && config->version >= 5) {
+#if defined(PDF_USE_SKIA)
+    CHECK(config->m_FontLibraryType == FPDF_FONTBACKENDTYPE_FREETYPE ||
+          config->m_FontLibraryType == FPDF_FONTBACKENDTYPE_FONTATIONS);
+    if (config->m_FontLibraryType == FPDF_FONTBACKENDTYPE_FONTATIONS) {
+      CHECK_EQ(renderer_type, CFX_GEModule::RendererType::kSkia);
+      backend = CFX_FontMgr::FontBackend::kFontations;
+    }
+#else
+    // AGG-only builds should always use `FPDF_FONTBACKENDTYPE_FREETYPE`.
+    CHECK_EQ(config->m_FontLibraryType, FPDF_FONTBACKENDTYPE_FREETYPE);
+#endif
+  }
+
   CFX_GEModule::Create(config ? config->m_pUserFontPaths : nullptr,
                        renderer_type, backend);
 
@@ -240,6 +251,7 @@
     return;
   }
 
+  // Note: we teardown/destroy things in reverse order.
   IJS_Runtime::Destroy();
 
 #ifdef PDF_ENABLE_XFA
```

Change information

Commit message:

Move renderer selection from CFX_DefaultRenderDevice to CFX_GEModule.

Consolidate initialization into a single CFX_GEModule::Create() call.
This centralizes global engine state and simplifies the library
initialization workflow.



-- Avoid runtime test for skia if not compiled into PDFium.

Change-Id: I7c737e83969d3e2aaaf9528698739fa1392212b2

Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/144752

Reviewed-by: Lei Zhang <the...@chromium.org>

Commit-Queue: Tom Sepez <tse...@chromium.org>

Files:

M core/fpdfapi/page/cpdf_pageimagecache.cpp
M core/fpdfapi/render/cpdf_imagerenderer.cpp
M core/fpdfapi/render/cpdf_renderstatus.cpp
M core/fxge/cfx_defaultrenderdevice.cpp
M core/fxge/cfx_defaultrenderdevice.h
M core/fxge/cfx_gemodule.cpp
M core/fxge/cfx_gemodule.h
M core/fxge/cfx_glyphcache.cpp
M core/fxge/cfx_renderdevice.cpp
M core/fxge/dib/cfx_dibitmap.cpp
M core/fxge/skia/fx_skia_device.cpp
M core/fxge/skia/fx_skia_device_embeddertest.cpp
M core/fxge/win32/cgdi_device_driver.cpp
M fpdfsdk/cpdfsdk_helpers.cpp
M fpdfsdk/fpdf_formfill.cpp
M fpdfsdk/fpdf_progressive.cpp
M fpdfsdk/fpdf_view.cpp
M fpdfsdk/fpdf_view_embeddertest.cpp
M testing/embedder_test.cpp
M testing/pdf_test_environment.cpp
M xfa/fde/cfde_textout_unittest.cpp
M xfa/fwl/cfwl_edit_embeddertest.cpp

Change size: L

Delta: 22 files changed, 162 insertions(+), 156 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Lei Zhang

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

open

diffy

satisfied_requirement

Reply all

Reply to author

Forward

Move renderer selection from CFX_DefaultRenderDevice to CFX_GEModule. [pdfium : main]

Tom Sepez (Gerrit)

Tom Sepez voted Commit-Queue+1

Related details

Lei Zhang (Gerrit)

Lei Zhang added 4 comments

Related details

Lei Zhang (Gerrit)

Lei Zhang voted Code-Review+1

Related details

Tom Sepez (Gerrit)

Tom Sepez voted and added 3 comments

Votes added by Tom Sepez

3 comments

Related details

Tom Sepez (Gerrit)

Tom Sepez voted Commit-Queue+2

Related details

Pdfium LUCI CQ (Gerrit)

Pdfium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

Change information