Fix set_font() for PDF and SVG backends#674
Conversation
| if not os.path.splitext(filename)[-1] in (".ttf", ".ttc"): | ||
| raise | ||
|
|
||
| pdfmetrics.registerFont(TTFont(face_name, filename)) |
There was a problem hiding this comment.
As far as I can tell, this embeds the font in the PDF file. I'm not sure if we want that or not.
PDF has a set of "safe" fonts which come with Acrobat Reader. I think maybe what we want to do in this method is try to map all requested fonts to the nearest match in this list.
There was a problem hiding this comment.
As far as I can tell, this embeds the font in the PDF file.
Based off the link below, I believe this is true as well.
PDF has a set of "safe" fonts which come with Acrobat Reader. I think maybe what we want to do in this method is try to map all requested fonts to the nearest match in this list.
I am unsure about this / why it would be necessary, but my understanding of all this is very much not complete / still growing. When would embedding the font in the PDF file not work or be problematic?
ref: https://www.reportlab.com/docs/reportlab-userguide.pdf
pg. 28 and 49-52
There was a problem hiding this comment.
When would embedding the font in the PDF file not work or be problematic?
You're right. It's probably OK.
aaronayres35
left a comment
aaronayres35
left a comment
There was a problem hiding this comment.
LGTM
I re-ran the script and pdf/svg no longer fail for the show_text benchmark. These are the results I see:
Admittedly I am not 100% confident in my understanding of all the details of the changes made. Nonetheless, this PR certainly seems to be an incremental improvement, and from what I do understand the changes make sense.
| if not os.path.splitext(filename)[-1] in (".ttf", ".ttc"): | ||
| raise | ||
|
|
||
| pdfmetrics.registerFont(TTFont(face_name, filename)) |
There was a problem hiding this comment.
As far as I can tell, this embeds the font in the PDF file.
Based off the link below, I believe this is true as well.
PDF has a set of "safe" fonts which come with Acrobat Reader. I think maybe what we want to do in this method is try to map all requested fonts to the nearest match in this list.
I am unsure about this / why it would be necessary, but my understanding of all this is very much not complete / still growing. When would embedding the font in the PDF file not work or be problematic?
ref: https://www.reportlab.com/docs/reportlab-userguide.pdf
pg. 28 and 49-52
|
Thanks for the review |
3 participants
Font lookup was a bit screwy with all of the file-based Kiva backends. This fixes
set_fontfor PDF and SVG, at least with respect to the benchmark program.