Merge spans, simplify applyLinePadding and fix fillLineGap behaviour

[nigelmegitt]

This pull request does several things:

1. Adjacent character spans are merged together into a single span. This means that if e.g. screen readers read the text they do not read each letter separately. It also means that user agents that break ligatures at span boundaries should get an improved rendering behaviour.
2. Simplifies applyLinePadding to use padding on the line end spans.
3. Fixes fillLineGap, closing semi-opaque backgroundColor and fillLineGap don't play nicely #200. Can be used to generate a test image for Create 50% transparency fillLineGap005 test w3c/imsc-tests#100

When comparing renders of the tests, I got this result:

$ python script/refpngcompare.py sandflow/imscJS/renders/imsc1/renders/png/ tests_render/imsc1/renders/png/
/Users/megitn02/Code/sandflow/imscJS/renders/imsc1/renders/png/FillLineGap002/0.000000.png: 0.000225907
/Users/megitn02/Code/sandflow/imscJS/renders/imsc1/renders/png/FillLineGap004/0.000000.png: 0.000468971
/Users/megitn02/Code/sandflow/imscJS/renders/imsc1/renders/png/FillLineGap003/0.000000.png: 0.000728906
/Users/megitn02/Code/sandflow/imscJS/renders/imsc1/renders/png/FillLineGap003/35.000000.png: 0.00154229
/Users/megitn02/Code/sandflow/imscJS/renders/imsc1/renders/png/FillLineGap003/30.000000.png: 0.00171816
/Users/megitn02/Code/sandflow/imscJS/renders/imsc1/renders/png/FillLineGap003/5.000000.png: 0.000773408
/Users/megitn02/Code/sandflow/imscJS/renders/imsc1/renders/png/FillLineGap003/20.000000.png: 0.00144757
/Users/megitn02/Code/sandflow/imscJS/renders/imsc1/renders/png/FillLineGap003/25.000000.png: 0.00014814
/Users/megitn02/Code/sandflow/imscJS/renders/imsc1/renders/png/FillLineGap003/15.000000.png: 0.000651107
/Users/megitn02/Code/sandflow/imscJS/renders/imsc1/renders/png/FillLineGap003/10.000000.png: 0.00137215
/Users/megitn02/Code/sandflow/imscJS/renders/imsc1/renders/png/LinePadding005/0.000000.png: 0.0062804
/Users/megitn02/Code/sandflow/imscJS/renders/imsc1/renders/png/linePadding3/0.000000.png: 0.00014617
/Users/megitn02/Code/sandflow/imscJS/renders/imsc1/renders/png/FillLineGap001/0.000000.png: 0.000507626
/Users/megitn02/Code/sandflow/imscJS/renders/imsc1/renders/png/DisplayAlign004/0.000000.png: 0.000578269
$ python script/refpngcompare.py ~/Code/sandflow/imscJS/renders/imsc1.1/renders/png/ tests_render/imsc1.1/renders/png/
/Users/megitn02/Code/sandflow/imscJS/renders/imsc1.1/renders/png/shear001/0.000000.png: 0.000475705
/Users/megitn02/Code/sandflow/imscJS/renders/imsc1.1/renders/png/shear001/5.000000.png: 0.000519006
/Users/megitn02/Code/sandflow/imscJS/renders/imsc1.1/renders/png/shear001/2.000000.png: 0.000521545
/Users/megitn02/Code/sandflow/imscJS/renders/imsc1.1/renders/png/shear001/4.000000.png: 0.000490956
/Users/megitn02/Code/sandflow/imscJS/renders/imsc1.1/renders/png/shear001/1.000000.png: 0.000475886
/Users/megitn02/Code/sandflow/imscJS/renders/imsc1.1/renders/png/shear001/3.000000.png: 0.00047467

On inspection, I couldn't see anything that would be especially worrisome about these diffs. The largest diff, for LinePadding005/0.000000.png is mainly caused by the correct rendering of ligatures in the Arabic text, demonstrating a benefit of this pull request.

[nigelmegitt]

Just noticed that linePadding005 is in fact showing the wrong behaviour, and I need to fix by putting back the negative margin that somehow got pulled out before.

[Update: absence of negative margin was fixed, this comment no longer applies.]

[palemieux]

@nigelmegitt Thanks for the all the effort. This looks super promising. What about linePadding3?

[nigelmegitt]

Thanks for the pointer @palemieux ; weirdly that quite big visual difference gave a tiny numerical value from the image comparison tool so I didn't look at it in detail. I can reproduce the difference, and will investigate.

[nigelmegitt]

Having looked at this, I think there's a discussion to be had about which is the correct render of linePadding3. The line above "Two lines" is generated by the preserved line break in the character content of the <span style="purpleStyle" xml:space="preserve"> that precedes the first child span, and the visible purple area is created by linePadding being applied to the empty span created for that line, which adds left and right padding.

The question I have is: does that preserved line break generate a line area? This is a similar case to #190, where the conclusion was that an empty <br/> does not create a line area. If we consider this preserved line break to be synonymous with a <br/> then there is no line area, just a spacing, and the linePadding (and in the case of #190, the line gap filling padding) should not be applied.

[nigelmegitt]

Another data point: inspecting the generated <span>, the contents of its innerHTML are "\n" suggesting it is not considered truly empty.

As an experiment, replacing the preserved new line with a <br/> creates the original rendering, where there is in fact no corresponding span in the HTML for the blank line between "No spaces" and "Two lines...".

My conclusion is that the linePadding3 test content creates a zero length line area, to which linePadding should be applied, therefore this code fixes a bug that had not previously been identified. Happy to hear counter-arguments!

[nigelmegitt]

Having thought about this overnight, I think the intent of the linePadding feature is to make text more readable by adding background padding on the ends of lines of text. In this case, of linePadding3, there is no visible text, so there is no readability benefit to drawing a weird empty background blob. Hence I pushed 31ae441 which does not apply line padding when the line has a single span whose bounding client rect has zero width or zero height.

[nigelmegitt]

Quick overview of the detail of this PR:

span merging algorithm:

1. In processElement(), when processing character text children of spans, for each span that is generated, record the corresponding element in the ISD from where it originated.
2. After applying linePadding, for each line:
   1. merge the adjacent spans on each line that each had the same originating ISD element back together into a single span: since they all came from the same source, they have the same styles, unless they are at the beginning or end of a line, in which case
   2. copy across any border, padding and margin styles that may have been applied to the relevant edge.
3. For each resulting span on each line, set background-color to the background color property applied to the closest span ancestor that specifies a background color, if any, and store what that span ancestor was.
4. For each span ancestor whose background color we applied to a descendant element, clear the background-color property.

The purpose of merging the spans is primarily to ensure that screen readers and layout engines treat blocks of adjacent characters as belonging together; it also provides a small (so far) performance improvement because there are fewer elements over which to iterate in later steps. Currently the only later step is applyFillLineGap() but a good follow-up to this pull request would be to see how much earlier we can merge the spans without causing issues.

The purpose of steps 3 and 4 is to ensure that the background color is applied to the span that has the correct size (including padding), which is relevant to linePadding and fillLineGap.

line padding

applyLinePadding() has the following changes:

• Don't apply linePadding if there is exactly one child element on the line and it has either zero width or zero height.
• When applying linePadding, do it by setting the padding-* property on the relevant edges (previously border was used). This extends the background correctly but has the effect of shifting the text; To offset that, set a corresponding negative value of the padding length as margin-* property so that the net result is the correct placement.

Note that the parent p element already has padding applied to inset the text prior to line layout to accommodate the linePadding and ensure that later application of linePadding does not cause unwanted line wraps.

line gap filling

applyFillLineGap() has the following changes:

• Round the frontier so that when the relevant padding is computed on each span on each line, it takes the value to an integer pixel location. By experimentation this was found to remove any overlaps and gaps between adjacent line area backgrounds, particularly visible with semi-transparent background colors.
• Calculate and apply the required padding on the appropriate edges separately for each span in each line. Do not round this value, since rounding frontier already achieved what we need.