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.

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

Fixed fonts in the generated file after usiong a reader #793

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ag3202
Copy link

@ag3202 ag3202 commented Feb 28, 2024

Discovered a font issue in the scenario of importing and then exporting, not sure about other scenarios. Unclear about the handling of Panose length and adding zeros, as Panose can easily exceed 10, and not very familiar with using dechex. After fixing, the font displays correctly, placing a:latin in a:ea results in the font displaying correctly after modification.

@Progi1984 Progi1984 changed the base branch from develop to master September 1, 2024 10:52
@xlar8or
Copy link

xlar8or commented Jan 3, 2025

I'm experiencing this issue, it's throwing the exception because the panose is already 020B0604020202020204 instead of 2B64222224. Instead of removing the len check we could check if it's 20 before throwing the exception, if it is, it runs a logic that cuts the padded zeros.

@Progi1984
Copy link
Member

@xlar8or Could you rebase your PR against the master branch, please? Thanks

@xlar8or
Copy link

xlar8or commented Jan 15, 2025

@xlar8or Could you rebase your PR against the master branch, please? Thanks

Seems to work, thanks! I'm stil having having issues where the save of a pptx is broken (vertical text instead of horizontal text, wrong font size, shapes aren't saved, image sizes aren't the same, audio/video isn't saved, etc), so it doesn't seem to be related with panose. I've already commented on this issue in #837 so hopefully you'll be able to look at that soon, because due to this issue it unfortunantely makes the library unusable to output a pptx that isn't fully made with the library functions and uses slides from another pptx.

@Progi1984
Copy link
Member

Yeah. I would like to merge this PR for fixing font issues and continue on issues next week.

So i need you rebase your PR with the font issue fix.

@xlar8or
Copy link

xlar8or commented Jan 15, 2025

Yeah. I would like to merge this PR for fixing font issues and continue on issues next week.

So i need you rebase your PR with the font issue fix.

I've replaced my entire library with the latest master and the panose fix works, so that's all good on my side. I didn't do the PR that was @ag3202.

@Progi1984
Copy link
Member

@xlar8or So sorry

@ag3202 Could you rebase it ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants