Provide all text state operators #1216
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Provide textRenderingMode, horizontalScaling, textRise, charcterSpacing and wordSpacing as an option for drawing text. In operator.ts and PDFOperatorNames.ts those operators were already implementes. The function for the horizontalScaling (as it is called in the pdf standard) was called "setCharacterSqueeze" so far. To keep the standardized terminology, the function was renamed to "setHorizontalSpacing". However, for backward compatibility (if somebody had used setCharacterSqueeze in his own script; in the actual pdf-lib code there was no reference) the original function was kept as well with a deprecated note. In the textOptions, not the standard names were used, but noun forms of it, to match the current options style (e.g. horizontalScale (options) instead of horizontalScaling (pdf standard)).
This reverts commit 39386a8.
revert temporary test removal
the first commit in master had to be reverted, since the tests were still in a temporary modified state. A new branch (textOptions) was then created on the basis of the meanwhile reverted commit. In this branch, the tests were cleaned up and the branch finally got merged with tha master. However, the actual changes, which were reverted in the master, were not merged, despiute the fact they were present in the brach. Finally, I manually added the actual changes directly in the master.
harmonization of tests + linting
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
The pdf specification allows several text state operators, e.g. for the font, size (Tf) and the line distance (leading, TL). However, there are some more (text rise, horiztontal scaling, word spacing, character spacing, text rendering mode) and I want to make use especially of the horizontalScaling (Tz). I saw that in the OperatorNames actually all were already defined, but not provided to the user (e.g. in drawText) yet. Therefore, I extended everything needed so that all text state operators can be set in the options for drawText. All those operators are implemented to be written only if the option is actually set, i.e. no default values are written to pdf, which would make the file unnecessarily larger.
Example:
page.drawText('hello world', { y: size - 40, size: 20, lineHeight: 20, horizontalScale: 50, rise: 5, wordSpace: 10, renderingMode: TextRenderingMode.Outline, characterSpace: 4, });Why?
I will need horizontal text scaling. Other users might want to use this or any of the other 4 added text state operators (word spacing, character spacing, text rise, text rendering mode) as well.
How?
It is implemented in the exact sam way as setting the font (Tf) or the lineHeight (TL). It is the most logic implementation and follows the present code structure. The horizontal spacing was actually already implemented not just in OperatorNames, but also the operator was already written in the name of "setCharacterSqueeze". In my opinion, it would be nicer if the operator would use the name as in the standard (i.e. setHorizontalScaling), as it is the case for all the other operators I have looked at. Therefore, created an additional operator "setHorizontalScaling", but kept the old "setCharacterSqueeze" one for backwards compatibility (with a deprecated note in the docstring). (Note: setCharacterSqueeze was used neither in the code nor in the tests (at least not uncommented), but users might by using it "the hard way", bypassing the "drawText"-function.
Testing?
Yes.
Unit tests: All pass.
Integration tests: added a page in test 1 with the new options. All pass. pdf looks ok in Adobe, Foxit, Chrome, Firefox.
Linting successful.
Typecheck successful.
The changes to the code can hardly have an impact to any existing code, since it only extends the existing part. The added options are working as they should, which can be seen in the last page of test1.
New Dependencies?
No.
Screenshots
N/A
Suggested Reading?
Yes
Anything Else?
I did not add any unit tests, since there were no unit tests for the other text state operators and I haven't created any unit tests so far (being just a hobby coder). For this reason, I could not just create an analogous test for the added text state options.
Checklist