StrutStyle for leading/lineheight adjustment on ParagraphStyle + TextStyle.setLetterSpacing and TextStyle.setWordSpacing

[stenson]

Not sure if there’s already a plan to add this in the works, but this PR adds a minimal version of textlayout.StrutStyle with .setLeading for modifying inter-line spacing with textlayout.Paragraph.

A code example (to modify https://github.com/HinTak/skia-python-examples/blob/main/skparagraph-example.py to get double-spacing):

strut_style = textlayout.StrutStyle()
strut_style.setStrutEnabled(True)
strut_style.setLeading(2.0) # 1.0 is the default

para_style = textlayout.ParagraphStyle()
para_style.setStrutStyle(strut_style)

[HinTak]

Thanks for the code. Too many things to do, so always good to have others adding stuff.

Btw, this will fail CI unless you pull in #296 , which is the fix for #295 - python 3.13.1 breaks.

[HinTak]

Argh, note to myself - this will collide with #293 in the next merge. Just slight inconvenience.

[stenson]

@HinTak thanks for taking a look! Happy to help where I can. Hopefully I pulled in the #296 change properly. I added more tests and also added TextStyle.setLetterSpacing and TextStyle.setWordSpacing to round out some things I was hoping to see in the Paragraph implementation. I figured it made sense to bundle these rather than submit a bunch of very small PRs that potentially collide with other work you’re doing here. If that’s not the case let me know and I can re-do everything as three different PRs.

[HinTak]

That's fine - indeed it is easier if you put in more changes in related area in one pull, or even small unrelated ones for that matter. As long as they are useful & a few more tests get added.

[HinTak]

@stenson I meant modifying the tests to look something like this:

@pytest.mark.parametrize('enable_strut, leading_value', [
    (True, 2.0),
    (True, 3.0),
    (True, 0.5),
    (True, 0),
])
def test_Paragraph_strutHeight(paragraph_builder, textlayout_text_style, textlayout_font_collection, paragraph_style, strut_style,
                               enable_strut, leading_value):
...
...
    if (leading_value > 1.0):
        assert  graf_with_strut(enable_strut, leading_value).Height > nostrut_height
    else:
    # Anything < 1 seems to have no effect
        assert  graf_with_strut(enable_strut, leading_value).Height == nostrut_height

This is neater, don't you think? Also, this counts as 4 tests (each of them are allowed to behave differently), rather than 1 (stopping at the first failure).

Have a go at adapting the test_Paragraph_wordSpacing and test_Paragraph_letterSpacing?

[stenson]

@HinTak thanks for the example! That definitely makes sense and I'm happy to rewrite the tests, I'm just not sure I see how to convert the existing tests, as they all rely on capturing and comparing relative results like this:

    negative_one_x_letter_spacing = graf_with_letterspacing(-1.0).LongestLine
    zero_x_letter_spacing = graf_with_letterspacing(0.0).LongestLine

    assert zero_x_letter_spacing > negative_one_x_letter_spacing

Is it preferable to have the tests test for specific values then? i.e. testing that the LongestLine is pytest.approx(80.42) rather than comparing different letter spacing's as relative values

[HinTak]

@HinTak thanks for the example! That definitely makes sense and I'm happy to rewrite the tests, I'm just not sure I see how to convert the existing tests, as they all rely on capturing and comparing relative results like this:

    negative_one_x_letter_spacing = graf_with_letterspacing(-1.0).LongestLine
    zero_x_letter_spacing = graf_with_letterspacing(0.0).LongestLine

    assert zero_x_letter_spacing > negative_one_x_letter_spacing

Is it preferable to have the tests test for specific values then? i.e. testing that the LongestLine is pytest.approx(80.42) rather than comparing different letter spacing's as relative values

I thought that's straightforward? you could do something like this:

# comparing results from pairs of values
@pytest.mark.parametrize('spacing_A, spacing_B', [
    (-1.0, 0),
...
])
...
...
... # note any exceptions about negative spacings etc
if (spacing_A > spacing_B):
    assert graf_with_letterspacing(spacing_A).LongestLine >  graf_with_letterspacing(spacing_B).LongestLine
else:
# the other way round

[HinTak]

As for what values to test - I think it is nice to test unusual / surprise values if only to document what they should be (and later detect if things change upstream), or if they misbehave, so zero / negatives are always interesting.

Looks like strut has the same meaning in age old LaTeX - it is an invisible(zero width?) box of a certain height, so height less than 1 does not affect line spacings.

[HinTak]

It is a single character in LaTeX though, https://en.m.wikipedia.org/wiki/Strut_(typesetting) not a paragraph property.

[stenson]

@HinTak that makes sense, I've updated all three new tests here to with pytest.mark.parametrize

[HinTak]

Cool. Thanks for the work. I'll add this to the m134 pile - m133 was out a few days ago, so m134 should be in about a month's time.

[stenson]

@HinTak that sounds great — thanks for the review