Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow embedding CFF fonts #1282

Closed
wants to merge 2 commits into from
+16 −7
Closed

Allow embedding CFF fonts #1282

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
09a9840
Allow embedding Type1C fonts, i.e. fonts with a CFF table.
camertron Dec 28, 2022
8239ac5
Merge upstream
camertron Jan 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions lib/prawn/fonts/ttf.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,6 @@ def embed(reference, subset)
descriptor = @document.ref!(
Type: :FontDescriptor,
FontName: basename.to_sym,
FontFile2: fontfile,
FontBBox: bbox,
Flags: pdf_flags,
StemV: stem_v,
Expand Down Expand Up @@ -351,8 +350,8 @@ def embed(reference, subset)
range_blocks =
ranges.reduce(+'') do |s, list|
s << format(
"%<lenght>d beginbfchar\n%<list>s\nendbfchar\n",
lenght: list.length,
"%<length>d beginbfchar\n%<list>s\nendbfchar\n",
length: list.length,
list: list.join("\n")
)
end
Expand All @@ -364,14 +363,22 @@ def embed(reference, subset)
cmap.stream.compress!

reference.data.update(
Subtype: :TrueType,
BaseFont: basename.to_sym,
FontDescriptor: descriptor,
FirstChar: 32,
LastChar: 255,
Widths: @document.ref!(widths),
ToUnicode: cmap
)

if font.cff.exists?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be useful/more readable to have this extracted into a separate method and have an OTF subclass that overrides it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a great idea 👍

Copy link
Member Author

@camertron camertron Jan 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thought, I think this is correct as-is. It appears Prawn looks at the file extension to determine which Font subclass to use. That's not wrong necessarily, but nothing prevents fonts with a .ttf file extension from having a CFF table. I've also seen this in practice. Checking for the existence of the table is, IMHO, less error-prone.

On that note, I've also (weirdly) come across fonts with both glyf and CFF tables. No idea how that's supposed to work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right that file extension is used. It's an easy heuristic. Do you think we should change that to make it more sophisticated and closer to "technically correct"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do. OTF and TTF aren't really different formats, they just contain different tables. Prawn would only have to read the directory structure at the beginning of the font to know if it contains a CFF table. Is that something you'd like to see in this PR?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've also (weirdly) come across fonts with both glyf and CFF tables. No idea how that's supposed to work.

Maybe it's a hybrid font? You can treat it as TTF or OTF and still get the same result? We should look into a few examples to verify if that's the case at least for the majority of cases. If that's so we should probably treat those as OTF fonts.

Is that something you'd like to see in this PR?

Yes, please.

And maybe add a changelog entry?

fontfile.data.update(Subtype: :Type1C)
reference.data.update(Subtype: :Type1)
descriptor.data.update(FontFile3: fontfile)
else
reference.data.update(Subtype: :TrueType)
descriptor.data.update(FontFile2: fontfile)
end
end

UNICODE_CMAP_TEMPLATE = <<-STR.strip.gsub(/^\s*/, '')
Expand Down
8 changes: 5 additions & 3 deletions spec/prawn_manual_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,18 @@
MANUAL_HASH =
case RUBY_ENGINE
when 'ruby'
'2c0279e0bff2a9120494a52aa46216c1871902b5e66a3537bd4d3cbd66db0096b43b6e1ae0e4e189b561c4db9fa1afacb6c41f260e3aaf942faae2fee352d35b'
'85e288ee68c956b0144bf32559b69eb1eae0e4c05e41436299eb8e91f52872f0'\
'd29272257b8cd79daa1033de73aadaf0c68d7744a23fe9fda86f45c9871748f4'
when 'jruby'
'51baf6440907e9e38d22f50deafa91572aec1174e621c044ae077cfe3d4361982a505dae5f013dd06f64f38cb9b3a38d5a3f8f0903849591774e298a3c91d39a'
'a3dcec0745e16985fa2713dcaa6a1186b1a6a8c97096a3923893eebf128377ce'\
'01228abf4d7335ae1e55984c0e973c4a57a036dd58345c6526cd082c0148082a'
end

RSpec.describe Prawn do
describe 'manual' do
# JRuby's zlib is a bit quirky. It sometimes produces different output to
# libzlib (used by MRI). It's still a proper deflate stream and can be
# decompressed just fine but for whatever reason compressin produses
# decompressed just fine but for whatever reason compression produces
# different output.
#
# See: https://github.com/jruby/jruby/issues/4244
Expand Down