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

Fix: push/restore (q/Q) losts selected font #348

Closed
wants to merge 1 commit into from

Conversation

mcerveny
Copy link

@mcerveny mcerveny commented Mar 5, 2022

Problem:

  • before text is rendered and self.fill_color!=self.text_color whole context is pushed (q)
  • when font is changed, font change is generated and remembered (self.font_style)
  • after text is rendering whole context (with old font) is restored (Q) but not restored in self.font_style
  • next rendering uses different "subset" table for different font

Proposed solution - restore only self.fill_color

Is this sufficient ?
Is the push/restore only for self.fill_color ?

test.pdf
test_bad.pdf

Test code:

import fpdf

pdf = fpdf.FPDF(orientation='portrait', unit='mm', format='A4', font_cache_dir='pdf/fonts/')
pdf.add_font('dejavusans', fname='pdf/fonts/DejaVuSans.ttf', uni=True)
pdf.add_font('dejavusans', style="B", fname='pdf/fonts/DejaVuSans-Bold.ttf', uni=True)
pdf.add_font('dejavusans', style="I",  fname='pdf/fonts/DejaVuSans-Oblique.ttf', uni=True)
pdf.add_font('dejavusans', style="BI", fname='pdf/fonts/DejaVuSans-BoldOblique.ttf', uni=True)
pdf.add_page()
pdf.set_font('dejavusans',size=10)
pdf.set_fill_color(255,0,0)
pdf.multi_cell(50, markdown=True,txt="aa bb cc **dd ee dd ee dd ee dd ee dd ee dd ee**")
pdf.output('test.pdf')

- before text is rendered and self.fill_color!=self.text_color whole context is pushed (q)
- when font is changed, font change is generated and remembered (self.font_style)
- after text is rendering whole context (with old font) is restored (Q) but not restored in self.font_style
-> next rendering uses different "subset" table for different font

Proposed solution - restore only self.fill_color

Signed-off-by: Martin Cerveny <m.cerveny@computer.org>
@mcerveny mcerveny requested a review from Lucas-C as a code owner March 5, 2022 22:17
@Lucas-C
Copy link
Member

Lucas-C commented Mar 7, 2022

Thanks for reporting this bug @mcerveny!

I'm not sure on what is the proper fix yet, but I'll look at it very soon.

As a starting point, here is a unit test that could be added to test_multi_cell_markdown.py, based on the code you provided:

def test_multi_cell_markdown_with_fill_color(tmp_path):  # issue 348
    pdf = fpdf.FPDF()
    pdf.add_page()
    pdf.set_font("Times", size=10)
    pdf.set_fill_color(255,0,0)
    pdf.multi_cell(50, markdown=True,txt="aa bb cc **dd ee dd ee dd ee dd ee dd ee dd ee**")
    assert_pdf_equal(pdf, HERE / "multi_cell_markdown_with_fill_color.pdf", tmp_path)

@Lucas-C Lucas-C added the bug label Mar 7, 2022
@Lucas-C Lucas-C closed this in 3549c0f Mar 7, 2022
@Lucas-C
Copy link
Member

Lucas-C commented Mar 7, 2022

I fixed that bug using a different approach in 3549c0f

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

Successfully merging this pull request may close these issues.

2 participants