astefanutti / decktape

PDF exporter for HTML presentations
MIT License
2.19k stars 176 forks source link

Font compression does not work for Emoji fonts #177

Closed DuaneOBrien closed 2 years ago

DuaneOBrien commented 5 years ago

After updating from 2.9.1 to 2.9.2, the PDF output by decktape has significantly increased in size.

Compare PDF from #176 : https://github.com/DuaneOBrien/decktape-emoji-bug/blob/master/decktape-emoji-bug-output.pdf : (132k in size)

To PDF generated from the same content post-update: https://github.com/DuaneOBrien/decktape-emoji-bug/blob/master/decktape-emoji-bug-output-post-update.pdf (14.8mb in size)

I have not yet tried trying --screenshots settings to see if this affects the output.

Chrome Version: 72.0.3626.121

astefanutti commented 5 years ago

I've tried with other presentations and the increase do not happen. So I highly suspect it's caused by the font compression algorithm failing to support Emoji that use zero width joiner sequences.

DuaneOBrien commented 5 years ago

Bummer!

Is this another bug that's further upstream? Anything I can do to help refine test cases, investigate, etc?

astefanutti commented 5 years ago

That one is likely on Decktape side. It's likely the way it aggregates font glyphes over slides that doesn't handle emoji correctly:

https://github.com/astefanutti/decktape/blob/46b23e53858c4c29a2027096142effcbc8ea82ee/decktape.js#L407-L452

Your presentation is enough to reproduce and test the future fix.

DuaneOBrien commented 5 years ago

Hi @astefanutti!

I opened #196 and #197 after I started looking at this bug a little closer. I'd like to see if I can get this one fixed, or at least understand it more, but I wanted to make sure that I understood what testing the fix would look like. Based on what I see in the repository, my plan for testing a fix would be to:

Does this look like a sane approach to you? Is there anything else that might be helpful to know before I try fixing the issue?

astefanutti commented 5 years ago

Hi @DuaneOBrien, this is just perfect! This is pretty much the protocol I use, that is relying on the examples as input for non-regression testing. Granted that, ideally, this should be automated, but this would require a substantial amount of work 😉.

Do not hesitate to report here your progress and questions so that I can help anytime.

astefanutti commented 2 years ago

I've tested the issue with the latest 3.4.0, and it seem it's fixed. Here is the output PDF: test.pdf, which is 290KB in size.

astefanutti commented 2 years ago

Let me close this as it seems to be fixed in recent versions.