Andy Phan (Gerrit)

unread,

Feb 18, 2026, 8:04:29 PM (4 days ago) Feb 18

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

Attention needed from Lei Zhang

Andy Phan added 1 comment

File core/fpdfapi/edit/cpdf_fontsubsetter.h

Line 54, Patchset 8: RetainPtr<const CPDF_Dictionary> root_font;

Andy Phan . unresolved

Can multiple RootFonts, CIDFonts, and FontDescriptors point to one font file?

Open in Gerrit

Related details

Attention is currently required from:

Lei Zhang

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

unsatisfied_requirement

open

diffy

Lei Zhang (Gerrit)

unread,

Feb 19, 2026, 4:47:18 PM (3 days ago) Feb 19

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

Attention needed from Andy Phan

Lei Zhang added 1 comment

File core/fpdfapi/edit/cpdf_fontsubsetter.h

Line 54, Patchset 8: RetainPtr<const CPDF_Dictionary> root_font;

Andy Phan . unresolved

Can multiple RootFonts, CIDFonts, and FontDescriptors point to one font file?

Lei Zhang

Probably yes. You can try creating a PDF like that and see if it works.

Open in Gerrit

Related details

Attention is currently required from:

Andy Phan

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

unsatisfied_requirement

open

diffy

Lei Zhang (Gerrit)

unread,

Feb 19, 2026, 5:17:50 PM (3 days ago) Feb 19

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

Attention needed from Andy Phan

Lei Zhang added 3 comments

File core/fpdfapi/edit/cpdf_fontsubsetter.h

Line 11, Patchset 9 (Latest):#include <random>

Lei Zhang . unresolved

Can this use core/fxcrt/fx_random.h instead?

File core/fpdfapi/edit/cpdf_fontsubsetter.cpp

Line 246, Patchset 9 (Latest): static constexpr auto kCharset = std::to_array<char>(

Lei Zhang . unresolved

Just generate some random numbers in the range [0, 25], and then add 'A'?

Line 253, Patchset 9 (Latest): if (base_font_name.GetLength() > kTagLength &&

Lei Zhang . unresolved

BTW, CPDF_SimpleFont::LoadCommon() has similar but not quite the same logic. There's also RemoveSubsettedFontPrefix() living inside core/fxge/cfx_fontmapper.cpp.

Open in Gerrit

Related details

Attention is currently required from:

Andy Phan

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

unsatisfied_requirement

open

diffy

Andy Phan (Gerrit)

unread,

Feb 19, 2026, 5:25:40 PM (3 days ago) Feb 19

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

Attention needed from Lei Zhang

Andy Phan added 1 comment

File core/fpdfapi/edit/cpdf_fontsubsetter.h

Line 54, Patchset 8: RetainPtr<const CPDF_Dictionary> root_font;

Andy Phan . unresolved

Can multiple RootFonts, CIDFonts, and FontDescriptors point to one font file?

Lei Zhang

Probably yes. You can try creating a PDF like that and see if it works.

Andy Phan

I haven't manually created a PDF yet, but FPDFText_LoadFont() creates another font file stream when given the exact same font data. With FPDF_SUBSET_NEW_FONTS only, the mapping is probably always 1:1.

If we implement subsetting all fonts, that will likely change.

Open in Gerrit

Related details

Attention is currently required from:

Lei Zhang

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

unsatisfied_requirement

open

diffy

Andy Phan (Gerrit)

unread,

Feb 20, 2026, 6:54:07 PM (2 days ago) Feb 20

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

Attention needed from Lei Zhang

Andy Phan added 3 comments

File core/fpdfapi/edit/cpdf_fontsubsetter.h

Line 11, Patchset 9:#include <random>

Lei Zhang . resolved

Can this use core/fxcrt/fx_random.h instead?

Andy Phan

Done

File core/fpdfapi/edit/cpdf_fontsubsetter.cpp

Line 246, Patchset 9: static constexpr auto kCharset = std::to_array<char>(

Lei Zhang . resolved

Just generate some random numbers in the range [0, 25], and then add 'A'?

Andy Phan

Done

Line 253, Patchset 9: if (base_font_name.GetLength() > kTagLength &&

Lei Zhang . resolved

BTW, CPDF_SimpleFont::LoadCommon() has similar but not quite the same logic. There's also RemoveSubsettedFontPrefix() living inside core/fxge/cfx_fontmapper.cpp.

Andy Phan

Using MaybeRemoveSubsettedFontPrefix() after moving in https://pdfium-review.googlesource.com/c/pdfium/+/143592.

Open in Gerrit

Related details

Attention is currently required from:

Lei Zhang

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

unsatisfied_requirement

open

diffy

Lei Zhang (Gerrit)

unread,

Feb 20, 2026, 7:31:26 PM (2 days ago) Feb 20

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

Attention needed from Andy Phan

Lei Zhang added 6 comments

File core/fpdfapi/edit/cpdf_fontsubsetter.h

Line 77, Patchset 10 (Latest): ByteString GenerateFontSubsetName(ByteString base_font_name);

Lei Zhang . unresolved

Can this live in the anonymous namespace now? Or be a const method?

Line 74, Patchset 10 (Latest): // prefixes if necessary. 9.6.4 Font Subsets: the font name must begin with a

Lei Zhang . unresolved

Write the ISO standard number here, as the paragraphs/sections are not always the same across different PDF spec versions.

Line 51, Patchset 10 (Latest): RetainPtr<const CPDF_Stream> font_stream;

Lei Zhang . unresolved

Put the new entries above this one, since if one starts traversing the objects from `root_font` at the top, the font file is one of the bottom leaf nodes.

Line 54, Patchset 8: RetainPtr<const CPDF_Dictionary> root_font;

Andy Phan . unresolved

Can multiple RootFonts, CIDFonts, and FontDescriptors point to one font file?

Lei Zhang

Probably yes. You can try creating a PDF like that and see if it works.

Andy Phan

I haven't manually created a PDF yet, but FPDFText_LoadFont() creates another font file stream when given the exact same font data. With FPDF_SUBSET_NEW_FONTS only, the mapping is probably always 1:1.
If we implement subsetting all fonts, that will likely change.

Lei Zhang

OK. Maybe leave a comment in the code as a reminder?

File core/fpdfapi/edit/cpdf_fontsubsetter.cpp

Line 245, Patchset 10 (Latest): ByteString tag;

Lei Zhang . unresolved

Call `tag.Reserve(kTagLength.+ base_font_name.GetLength() + 1);` since the final string length is known. Then keep appending to `tag` and return `tag`.

File core/fxcrt/fx_random.h

Line 22, Patchset 10 (Latest): // Chooses a random seed for the caller.

Lei Zhang . unresolved

Just use Fill() to generate some random numbers?

Open in Gerrit

Related details

Attention is currently required from:

Andy Phan

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

unsatisfied_requirement

open

diffy

Reply all

Reply to author

Forward

Add a prefix to font subset name [pdfium : main]

Andy Phan (Gerrit)

Andy Phan added 1 comment

Related details

Lei Zhang (Gerrit)

Lei Zhang added 1 comment

Related details

Lei Zhang (Gerrit)

Lei Zhang added 3 comments

Related details

Andy Phan (Gerrit)

Andy Phan added 1 comment

Related details

Andy Phan (Gerrit)

Andy Phan added 3 comments

Related details

Lei Zhang (Gerrit)

Lei Zhang added 6 comments

Related details