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

Skew text #627

Closed
wants to merge 4 commits into from
Closed

Skew text #627

wants to merge 4 commits into from

Conversation

erap129
Copy link

@erap129 erap129 commented Dec 14, 2022

Adding skew_text method (issue #536). Currently implemented it as a separate method. If you think this should be under local_context tell me and I will change it. Furthermore, we discussed about a "perspective" effect as well. I am pretty sure that it cannot be achieved with the Tm operator alone since it defines affine transformations and not projective transformations. It has to do with the two zeros in the Tm matrix which are unchangeable:
Screenshot from 2022-12-12 20-39-13
(figure taken from https://www.syncfusion.com/succinctly-free-ebooks/pdf/text-operators).

I applied math.tan on the angle as specified in the PDF specification document (page 126):
Screenshot from 2022-12-14 20-59-12
I added 1e-5 to the value of the angle (because otherwise using an angle of 90 would lead to no transformation at all, which seemed weird to me)

Open to comments and further suggestions for improvement.
Thanks!

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

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

Copy link
Member

@Lucas-C Lucas-C left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work Here!
Let's see if the CI pipeline pass 😊
Could you also add a mention of this great addition in CHANGELOG.md as part of this PR?

`skew_text` creates text that is skewed on the x or y axes:

```python
with pdf.skew_text(0, 10):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great doc!
Could you pass the parameters by name in this code sample, to be more explicit, please?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, ill fix that

fpdf/fpdf.py Show resolved Hide resolved
with doc.skew_text(89, 0):
doc.cell(txt="some extreme skewing")
doc.ln(40)
assert_pdf_equal(doc, HERE / "cell_skew_text.pdf", tmp_path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice unit test!
Could you also provite an extra small test with FPDF.multi_cell and some input text covering at least 2 lines?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK small problem. Just found out that _render_styled_text_line supports a fill option which generates rectangles around the text. They are graphics, and thus not affected by the Tm operator:
Screenshot from 2022-12-17 17-34-50
I thought of replacing it with a cm but then that would just be a skew function and not specifically skew_text.
I am guessing that I can limit the skewing only to the rectangle around the text with a few conditions in the _render_styled_text function (I'll also have to solve a weird problem where setting a skew on the x-axis creates a location offset as well, but that's probably solvable):
Screenshot from 2022-12-17 17-32-59
Another option of course would be not to support the fill option together with skewed text, but that may be limiting this feature.
What do you suggest?

Copy link
Member

@Lucas-C Lucas-C Dec 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of a generic skew function that would apply to not only text.
If you want to follow that road, that is really fine with me, and may be even more useful to fpdf2 users.

If you to stick to a skew_text method, you can just forbid it to be used with fill=True and raise a FPDFException or ValueError if it happens.

Copy link
Author

@erap129 erap129 Dec 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, after playing with it a bit I found that the transformation needed to fix the unwanted movement and leave only the skew is this (actually I just copied from rotation):

    @check_page
    @contextmanager
    def skew(self, ax=None, ay=None):
        lim_val = 2**32
        x = self.x
        y = self.y
        ax = max(min(math.tan(ax * (math.pi / 180)), lim_val), -lim_val)
        ay = max(min(math.tan(ay * (math.pi / 180)), lim_val), -lim_val)
        cx, cy = x * self.k, (self.h - y) * self.k
        with self.local_context():
            self._out(
                f"1 {ay:.5f} {ax:.5f} 1 {cx:.2f} {cy:.2f} cm "
                f"1 0 0 1 -{cx:.2f} -{cy:.2f} cm"
            )
            yield

So I guess I can implement a skew function instead of skew_text and it will be conceptually very similar to rotation, what do you say?

Copy link
Member

@Lucas-C Lucas-C Dec 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds great @erap129!
Go on, that will be a nice addition to fpdf2 😊

And "Joyeux Noël" as we say here in France ^^

@Lucas-C
Copy link
Member

Lucas-C commented Dec 15, 2022

If you think this should be under local_context tell me and I will change it.

I think a named method is great, and ensure finer control over the order of transformations applied
if skew is combined with some other operator.

I added 1e-5 to the value of the angle (because otherwise using an angle of 90 would lead to no transformation at all, which seemed weird to me)

This is really strange...
How do you explain that?
What skew parameters correspond to an angle of 90°?

@erap129
Copy link
Author

erap129 commented Dec 17, 2022

OK I understand the problem regarding an angle of 90. The tan function has asymptotes and thus math.tan(90 * (math.pi / 180)) leads to an insanely large number. I'll just change the function to this to avoid reaching this insane number:

@check_page
@contextmanager
def skew_text(self, ax=None, ay=None):
    lim_val = 2**32
    x = max(min(math.tan(ax * (math.pi / 180)), lim_val), -lim_val)
    y = max(min(math.tan(ay * (math.pi / 180)), lim_val), -lim_val)
    with self.local_context():
        self.skew = (y, x)
        yield

@Lucas-C
Copy link
Member

Lucas-C commented Dec 18, 2022

OK I understand the problem regarding an angle of 90. The tan function has asymptotes and thus math.tan(90 * (math.pi / 180)) leads to an insanely large number. I'll just change the function to this to avoid reaching this insane number:

Great, that's a nice solution!

@Lucas-C
Copy link
Member

Lucas-C commented Jan 2, 2023

Closing this in favor of #656

@Lucas-C Lucas-C closed this Jan 2, 2023
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.

2 participants