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.

Sign up for GitHub

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

Jump to bottom

actual line height is greater than specified lineHeight #201

Closed
nigelmegitt opened this issue Jan 14, 2021 · 1 comment · Fixed by #202
Closed

actual line height is greater than specified lineHeight #201

nigelmegitt opened this issue Jan 14, 2021 · 1 comment · Fixed by #202
Assignees
@palemieux
Labels
enhancement PR open
Milestone
1.1.2 Release

Comments

@nigelmegitt
Copy link
Contributor

Platform: Mac OS X
Browser: Firefox 84.0.1

Input

Sample IMSC document: 
<?xml version="1.0" encoding="UTF-8"?>
<tt ttp:cellResolution="40 24" ttp:timeBase="media" xml:lang="en-GB" xml:space="default" xmlns="http://www.w3.org/ns/ttml" xmlns:ebuttm="urn:ebu:tt:metadata" xmlns:ebutts="urn:ebu:tt:style" xmlns:itts="http://www.w3.org/ns/ttml/profile/imsc1#styling" xmlns:ttm="http://www.w3.org/ns/ttml#metadata" xmlns:ttp="http://www.w3.org/ns/ttml#parameter" xmlns:tts="http://www.w3.org/ns/ttml#styling" xmlns:xml="http://www.w3.org/XML/1998/namespace">
    <head>
        <metadata><ebuttm:documentMetadata><ebuttm:conformsToStandard>http://www.w3.org/ns/ttml/profile/imsc1/text</ebuttm:conformsToStandard><ebuttm:conformsToStandard>urn:ebu:tt:distribution:2018-04</ebuttm:conformsToStandard></ebuttm:documentMetadata></metadata>
        <styling>
            <style ebutts:linePadding="0.5c" tts:textAlign="start" xml:id="S1"/>
            <style tts:textAlign="center" xml:id="S2"/>
            <style tts:backgroundColor="#000000ff" tts:color="#ffffffff" xml:id="S3"/>
            <style tts:backgroundColor="#000000ff" tts:color="#00ffffff" xml:id="S6"/>
            <style itts:fillLineGap="true" xml:id="StyleFillLineGapTrue"/>
            <style tts:fontFamily="Arial, Roboto, proportionalSansSerif, default" xml:id="fontFamilyStyle"/>
            <style tts:fontSize="150%" tts:lineHeight="120%" xml:id="autogenFontStyle_n_150_120"/></styling>
        <layout>
            <region tts:displayAlign="after" tts:extent="70.375% 23.478%" tts:origin="17.625% 71.522%" tts:overflow="visible" tts:backgroundColor="#000080ff" xml:id="agr_3_18_37_6_after"/>
        </layout>
    </head>
    <body ttm:role="caption">
        <div style="autogenFontStyle_n_150_120 S1 StyleFillLineGapTrue fontFamilyStyle">
            <p region="agr_3_18_37_6_after" style="S2" xml:id="C27_1">
                <span style="S6">Moving on. We're in the</span>
                <br/>
                <span style="S6">millennium year and we need</span>
                <br/>
                <span style="S6">to commemorate the event. Any ideas?</span>
            </p>
        </div>
    </body>
</tt>

Render via http://sandflow.com/imsc1_1/index.html :

image

Problem:

In this document, we have a p with:

  • tts:lineHeight="120%"
  • tts:fontSize="150%"
  • where ttp:cellResolution="40 24"
  • and tts:fontFamily is (inherited from the div) "Arial, Roboto, proportionalSansSerif, default"

which computes to a line height of 100% * 1.2 * 1.5 / 24 = 7.5%
3 lines are visible, which should therefore take up 22.5% of the height.

The region height is 23.478%.

However what we see is that the region height is less than the height of the three stacked lines, rather than more.

What's going on in the browser?

Render area total height = 450px.

Height of one line is expected to be 7.5% of 450px = 33.75px
line-height property on the p is indeed set to 33.75px.
font-size of the text should be and is set to 28.125px - but on the p's child span, not on the p itself.

Outer span (child of p) computed height is 32px (for all the span children of p).
Inner span (child of outer span) computed height varies:

  • children of 1st outer span (1st line) have height 37px. (padding-top: 1px; padding-bottom: 4px;)
  • children of 2nd outer span (2nd line) have height 40px. (padding-top: 4px; padding-bottom: 4px;)
  • children of 3rd outer span (3rd line) have height 42px. (padding-top: 4px; padding-bottom: 6px;)

This all triggered me to do some experimenting, from which I learned that even though the line-height is being set on the p with an explicit length in px units, the browser overrides it. And it seems to depend on the used values of the font-family and font-size on the p element.

To make this more concrete, I prepared a CodePen at https://codepen.io/nigelmegitt/pen/qBaJZOv showing the effect.

TTML specifies that tts:fontFamily and tts:fontSize only apply to span elements and therefore not to p, with some text that explains that font details do matter when computing line height, but only when tts:lineHeight="normal" which is not the case here. So it seems completely reasonable for imscJS to set those attributes only on spans. However, if I do set them on the p then the result is the line height specified in the TTML.

Proposal

Arguably, for the purposes of reflecting real world implementations, tts:fontSize and tts:fontFamily should apply to p elements in TTML for the purpose of allowing implementations to take them into account when computing the line height. That would be a spec issue on TTML.

In the meantime, I would argue that:

  • imscJS should set the font-size and font-family properties on the p elements if they're there in the TTML, and
  • allow CSS inheritance rules to apply, as they do in TTML.

This would allow authorial control as if that spec change had been made, and does not appear to conflict with the spec as it is now. But it would also generate output behaviour that more closely matches the expectation when authoring TTML.

I'd be very interested in counter-arguments to this! Perhaps this approach was adopted before but resulted in other problems I have not noticed.
This was referenced Jan 14, 2021
Closed
Merged
@palemieux palemieux self-assigned this Feb 2, 2021
@palemieux palemieux added this to the 1.1.2 Release milestone Feb 2, 2021
@palemieux
Copy link
Contributor

palemieux commented Feb 2, 2021 •
edited

Per w3c/ttml2#1215, it looks like fontFamily and fontSize are applicable to <p>.

imscJS/src/main/js/styles.js

Line 188 in 5b39af7

['span'],

imscJS/src/main/js/styles.js

Line 252 in 5b39af7

['span'],

palemieux added a commit that referenced this issue Feb 2, 2021
@palemieux
Apply tts:fontFamily and tts:fontSize to <p> (#201)
37ea860
@palemieux palemieux mentioned this issue Feb 2, 2021
Merged
palemieux added a commit that referenced this issue Feb 3, 2021
@palemieux
Apply tts:fontFamily, tts:fontWeight, tts:fontStyle and tts:fontSize …
a0c3cdd 
…to p (#201)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@palemieux palemieux

Labels
enhancement PR open
Projects
None yet
Milestone
1.1.2 Release
Development

Successfully merging a pull request may close this issue.

Apply tts:fontFamily, tts:fontWeight, tts:fontStyle and tts:fontSize to p sandflow/imscJS
2 participants
@palemieux @nigelmegitt