skynav / ttt

Timed Text Toolkit
BSD 2-Clause "Simplified" License
75 stars 15 forks source link

Fix border and text overlapping (implements painting order recommendations from TTML2 spec) #311

Closed MaxEliaserAWS closed 2 years ago

MaxEliaserAWS commented 2 years ago

The TTML spec (TTML2 section 11.3.3.1 "painting order") refers to "the rendering model specified by XSL-FO 1.1, section 4.9." Referring to that XSL spec, section 4.9.6 ("Layering and Conflict of Marks,") it states "If A and B are areas with the same stacking layer, the backgrounds of A and B come beneath all other marks generated by A and B. Further, if A is an ancestor of B (still with the same stacking layer), then the background of A is beneath all the areas of B, and all the areas of B are beneath the intrinsic areas (and border) of A." I couldn't find any explicit language in the XSL spec about text outlines, but it makes sense to put them in a layer between background and text fill, as this produces the most pleasing rendering result (see examples below) and it at least in the spirit of the XSL spec to do it that way.

This code change was jointly authored by my colleague Nathan Hebert and by myself, with unit test additions and changes by myself.

MaxEliaserAWS commented 2 years ago

New Tests

I'll start by showing before-and-after outputs on the new unit tests I added, since this should illustrate the problem that this patch addresses.

ttml2-background-tall-characters-inline-lrtb

This is a version of ttml2-padding-inline-lrtb, using extra tall characters (large ascender/descender dimensions) with tight line spacing to encourage wrapped lines to overlap each other. This screenshot compares the output of this test with the current master code (bottom) vs. the patched code (top.) Both outputs do a good job of putting the main green background color for the paragraph behind all the characters, but in the latest master, the yellow background for the inline span is placed on top of some of the characters, partially obscuring them. There is also an inline space area with a red background color, showing the same problem. With the fixed code, because the background rectangles are in a separate layer, they do not obsucre the characters themselves. This test covers the background rendering codepaths in renderBlock, renderGlyphTextHorizontal, renderFiller, and renderSpace. ttml2-background-tall-characters-inline-lrtb

I also have tested the same thing in a vertical writing mode, but I have not included this test with this PR, as the latest master code has other serious issues that stop the test from rendering correctly (the characters don't advance in the inline progression direction and end up piled on top of each other.) I think that issue is best addressed in a separate PR.

ttml2-thick-text-outline-on-ruby-mixed

This is a version of ttml2-text-outline-on-ruby-mixed, where the text outline thickness has been increased from 0.11em to 0.4em. This screenshot compares the output of this test with the current master code (bottom) vs. the patched code (top.) As you can see, the outlines for character n are so thick that they parly cover the outlines for character n-1, meaning only the final character in each block area is fully visible. With the fixed code, because the character outlines are in a separate layer, they do not obscure the characters themselves. This test covers both of the codepaths that can generate text outlines, renderGlyphTextHorizontal and renderGlyphTextVertical. ttml2-thick-text-outline-on-ruby-mixed

Existing Tests

ttml2-text-outline-on-ruby-mixed

On this test case, the old and new versions render the same, for the most part. The old version is on bottom, the new version is on top. The difference is mainly in the SVG structure, with background (stroke) text all grouped together prior to the foreground (fill) text. ttml2-text-outline-on-ruby-mixed_comparison I also generated this SVG diff by pretty-printing both SVGs prior to diffing them. ttml2-text-outline-on-ruby-mixed.svg.diff.txt In this zoomed-in comparison, you can see one rendering difference: since the debug annotations are lumped in with the main layer, these now appear in front of the text backgrounds. In my opinion this improves the readability of the annotations, but I could put these in a separate layer if needed. In any case, without these developer-focused annotations toggled on, this input file would not render any differently without this change. ttml2-text-outline-on-ruby-mixed_annotation_comparison

ttml2-styling-issue-179

In this test case, the rendering results are identical for all three frames, with the only difference in the SVG structure. ttml2-styling-issue-179_frame1 ttml2-styling-issue-179_frame1.svg.diff.txt ttml2-styling-issue-179_frame2 ttml2-styling-issue-179_frame2.svg.diff.txt ttml2-styling-issue-179_frame3

ttml2-div-background-image-1

In this test case, the rendering result is identical, with the only difference in the SVG structure. In order to be absolutely sure of this, I made sure that the dat file was unchanged as well. ttml2-div-background-image-1.svg.diff.txt ttml2-div-background-image-1

ttml2-padding-inline-lrtb

In this test case, the rendering result is identical, with the only difference in the SVG structure. This one is notable because the background color rectangles are being moved, not text outlines. But it's the same principle. ttml2-padding-inline-lrtb.svg.diff.txt ttml2-padding-inline-lrtb

ttml1-prstn-set-duration-implicit ttml1-prstn-span-duration-implicit

For these test cases, the input files have a paragraph element that looks like this:

      <p begin="0s" end="1s" timeContainer="seq" tts:backgroundColor="blue">
        <span>This text should not be displayed.</span>
      </p>

There appears to be an unrelated issue that prevents the paragraph element from being pruned from the ISD, although the span element does get pruned. The result is that there are some empty group elements being inserted in the SVG prior to the "should be displayed" text, and that shows up in the SVG diffs. I don't think the root cause of this is in this PR, and the issue doesn't affect rendering results-- just drawing attention to it. ttml1-prstn-set-duration-implicit ttml1-prstn-set-duration-implicit.svg.diff.txt ttml1-prstn-span-duration-implicit ttml1-prstn-span-duration-implicit.svg.diff.txt

ttml1-prstn-anonymous-span-duration-implicit ttml1-prstn-break-duration-implicit

For these test cases, both the old and new outputs appear to be bugged the same way. The old and new outputs have an identical rendering result though, with the only differences being in the SVG structure. ttml1-prstn-anonymous-span-duration-implicit ttml1-prstn-anonymous-span-duration-implicit.svg.diff.txt ttml1-prstn-break-duration-implicit ttml1-prstn-break-duration-implicit.svg.diff.txt

ttml2-prstn-text-shadow-imsc11-1 ttml2-prstn-length-root-container-relative-imsc11-5

For these test cases, both the old and new outputs have the same defect (shadows not being rendered.) We do have a patch coming in the future that implements the tts:textShadow property, which will address this issue. In the meantime, for this PR, these are identical rendering results with the only differences being in the SVG structure. ttml2-prstn-text-shadow-imsc11-1 ttml2-prstn-text-shadow-imsc11-1.svg.diff.txt ttml2-prstn-length-root-container-relative-imsc11-5 ttml2-prstn-length-root-container-relative-imsc11-5.svg.diff.txt

ttml2-span-text-outline ttml2-text-outline-on-emphasis-mixed ttml1-prstn-background-color-inline-spaces-lrtb ttml1-prstn-text-outline ttml1-prstn-padding-region ttml1-prstn-animation ttml1-prstn-background-color-block ttml1-prstn-background-color-inline-lrtb ttml2-prstn-display-inline-block ttml2-prstn-length-root-container-relative-imsc11-4 ttml2-prstn-length-root-container-relative-imsc11-6 ttml2-prstn-extent-region-root-container-relative ttml2-prstn-ruby-reserve-imsc11-1 ttml2-prstn-ruby-reserve-imsc11-2 ttml2-prstn-ruby-reserve-imsc11-3

Identical rendering result, only difference in the SVG structure. I'm glossing over the rest of these, but I have comparison images and SVG diffs for all of them here: remaining_comparisons.zip

skynavga commented 2 years ago

LGTM

MaxEliaserAWS commented 2 years ago

Thanks Glenn!