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

Refactor: Move more text processing to line_break.py & char_spacing #511

Merged
merged 30 commits into from
Sep 3, 2022
Merged

Refactor: Move more text processing to line_break.py & char_spacing #511

merged 30 commits into from
Sep 3, 2022

Conversation

gmischler
Copy link
Collaborator

Checklist:

  • The GitHub pipeline is OK (green),
    meaning that both pylint (static code analyzer) and black (code formatter) are happy with the changes of this PR.
  • A unit test is covering the code added / modified by this PR
  • This PR is ready to be merged
  • In case of a new feature, docstrings have been added, with also some documentation in the docs/ folder
  • A mention of the change is present in CHANGELOG.md

I Instrumented Fragment() with more font and style information. The most effective way to do this turned out to attach a (shallow) copy of the current graphics context. This may currently look a bit redundant, but it will help implement flexible text flow regions later. It already simplifies the handling of markdown text a bit.
I also added the current value of self.k, While this is unlikely to change within a document, it is still needed for various conversions, so having it included with the fragment makes things a lot simpler.

Moved all text width calculations to Fragment(). With the help of the added font/style info it now automatically returns the stretched and char_spaced width, so that fpdf.py doesn't need to worry about that anymore.

Changed the API of MultiLineBreak() to receive and return document units instead of glyph space units. This eliminates a lot of conversions in fpdf.py. We now only need to deal with glyph space units there when we actually have to write them to the output file.

Everything in test_line_break.py had to get adapted to that API change.
Other than that, all tests are running through as before. Two PDFs needed updating, because the different sequence of unit conversions caused tiny differences in rounding (moving some text by an invisible amount).

Fixes #489
Since it has now become simpler to do, I implemented .set_char_spacing() as of #489. It follows the pattern of set_stretching(), just with an absolute value in Pica as the argument.

_render_styled_text_line() will eventually need some more refactoring. Flowing text regions will want to justify text with varying fonts and varying stretching and char spacing on the same line. This means that we'll first have to loop through the fragments and do the unicode check further in. But that's a task for another PR...

By submitting this pull request, I confirm that my contribution is made under the terms of the GNU LGPL 3.0 license.

@gmischler gmischler requested a review from Lucas-C as a code owner August 26, 2022 21:32
@codecov
Copy link

codecov bot commented Aug 26, 2022

Codecov Report

Merging #511 (41a18d4) into master (a14c93f) will increase coverage by 0.11%.
The diff coverage is 99.41%.

@@            Coverage Diff             @@
##           master     #511      +/-   ##
==========================================
+ Coverage   92.12%   92.23%   +0.11%     
==========================================
  Files          23       23              
  Lines        6793     6855      +62     
  Branches     1402     1410       +8     
==========================================
+ Hits         6258     6323      +65     
+ Misses        304      303       -1     
+ Partials      231      229       -2     
Impacted Files Coverage Δ
fpdf/line_break.py 98.98% <98.82%> (-0.32%) ⬇️
fpdf/fpdf.py 90.26% <100.00%> (+0.16%) ⬆️
fpdf/graphics_state.py 98.98% <100.00%> (+0.08%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

CHANGELOG.md Show resolved Hide resolved
docs/TextStyling.md Show resolved Hide resolved
docs/TextStyling.md Show resolved Hide resolved
docs/TextStyling.md Show resolved Hide resolved
fpdf/fpdf.py Show resolved Hide resolved
fpdf/fpdf.py Show resolved Hide resolved
fpdf/line_break.py Show resolved Hide resolved
fpdf/line_break.py Show resolved Hide resolved
fpdf/line_break.py Show resolved Hide resolved
fpdf/line_break.py Show resolved Hide resolved
@Lucas-C Lucas-C merged commit 1949838 into py-pdf:master Sep 3, 2022
@Lucas-C
Copy link
Member

Lucas-C commented Sep 3, 2022

Merged!
Thank you for this PR @gmischler

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

Successfully merging this pull request may close these issues.

Any idea how to achieve letter spacing?
3 participants