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

fillLineGap doesn't always fill the gap #253

Open
pjcherriman opened this issue Jan 12, 2024 · 12 comments
Open

fillLineGap doesn't always fill the gap #253

pjcherriman opened this issue Jan 12, 2024 · 12 comments

Comments

@pjcherriman
Copy link

pjcherriman commented Jan 12, 2024

I've notice a few very small linegaps between row of subtitles when the fiilLineGap option is used.

It is easier to see on content with a light background, but it can be seen if you look carefully with the BBC R&D test stream which is already in the DASH reference player.

stream: https://rdmedia.bbc.co.uk/testcard/simulcast/manifests/avc-ctv.mpd

Reference player & Stream: http://reference.dashif.org/dash.js/v4.7.3/samples/dash-if-reference-player/index.html?mpd=https%3A%2F%2Frdmedia.bbc.co.uk%2Ftestcard%2Fsimulcast%2Fmanifests%2Favc-ctv.mpd+&debug.logLevel=4&streaming.delay.liveDelayFragmentCount=NaN&streaming.delay.liveDelay=NaN&streaming.buffer.initialBufferLevel=NaN&streaming.liveCatchup.maxDrift=NaN&streaming.liveCatchup.playbackRate.min=NaN&streaming.liveCatchup.playbackRate.max=NaN

Attached are output on a window PC running full screen on a 4K monitor for both Chrome and Firefox, both show a linegap, and firefox is wrapping first line of subtitles for some reason.

chrome-linegap2

The linegap is most easily seen in the space between Musical and Scale where the yellow floodlight can be seen, or by zooming right in.

firefox-linegap-and-wrap2

On the firefox image, the linegap can be seen between the two musical notes.

The TTML similar to this
<?xml version="1.0" encoding="utf-8"?>
<tt xmlns="http://www.w3.org/ns/ttml" xmlns:ttp="http://www.w3.org/ns/ttml#parameter" xmlns:tts="http://www.w3.org/ns/ttml#styling" xmlns:ttm="http://www.w3.org/ns/ttml#metadata" xmlns:ebuttm="urn:ebu:tt:metadata" xmlns:ebutts="urn:ebu:tt:style" xmlns:itts="http://www.w3.org/ns/ttml/profile/imsc1#styling" xmlns:ittp="http://www.w3.org/ns/ttml/profile/imsc1#parameter" xml:lang="en" xml:space="default" ttp:timeBase="media" ttp:cellResolution="40 24" ittp:activeArea="33.500000% 7.500000% 33.000000% 60.500000%" >
	<head>
		<ttm:copyright>British Broadcasting Corporation</ttm:copyright>
		<metadata>
			<ttm:title>BBC DASH Testcard Stream</ttm:title>
			<ebuttm:documentMetadata>
				<ebuttm:conformsToStandard>urn:ebu:tt:distribution:2018-04</ebuttm:conformsToStandard>
				<ebuttm:conformsToStandard>http://www.w3.org/ns/ttml/profile/imsc1/text</ebuttm:conformsToStandard>
				<ebuttm:authoredFrameRate>25</ebuttm:authoredFrameRate>
				<ebuttm:documentOriginatingSystem>Perrott Subtitle Encoding</ebuttm:documentOriginatingSystem>
			</ebuttm:documentMetadata>
		</metadata>
		<styling>
			<style xml:id="S0" tts:wrapOption="wrap" itts:fillLineGap="true" tts:fontFamily="ReithSans, Arial, Roboto, proportionalSansSerif, default" ebutts:linePadding="0.5c" tts:textAlign="center" tts:fontSize="80%"/>
			<style xml:id="S1" tts:textAlign="start"/>
			<style xml:id="S10" tts:color="#FFFFFF" tts:fontSize="200%" tts:backgroundColor="#000000"/>
			<style xml:id="S11" tts:color="#FFFF00" tts:fontSize="200%" tts:backgroundColor="#000000"/>
			<style xml:id="S12" tts:color="#FF9010" tts:fontSize="200%" tts:backgroundColor="#000000"/>
			<style xml:id="S16" tts:fontStyle="italic" tts:color="#10FF10" tts:fontSize="200%" tts:backgroundColor="#000000"/>
		</styling>
		<layout>
			<region xml:id="R3" tts:origin="33.5% 32%" tts:extent="33% 36%" tts:overflow="visible" tts:displayAlign="before"/>
			<region xml:id="R8" tts:origin="35% 7.5%" tts:extent="30% 10%" tts:overflow="hidden" tts:displayAlign="before"/>
		</layout>
	</head>
	<body style="S0">
		<div region="R8">
			<p xml:id="p8-1-470" begin="473631:40:40.32" end="473631:40:44.16"><span style="S16">Top centre</span></p>
		</div>
		<div region="R3">
			<p xml:id="p3-6-470" style="S1"><span style="S10" begin="473631:40:40.32" end="473631:40:44.16">♫ Musical Scale ♪<br/></span><span style="S12" begin="473631:40:40.32" end="473631:40:44.16">C</span><span style="S12" begin="473631:40:40.32" end="473631:40:44.16">, D</span><span style="S12" begin="473631:40:40.32" end="473631:40:44.16">, E</span><span style="S12" begin="473631:40:40.32" end="473631:40:44.16">, F</span><span style="S12" begin="473631:40:40.32" end="473631:40:44.16">, G</span><span style="S12" begin="473631:40:40.5" end="473631:40:44.16">, A</span><span style="S12" begin="473631:40:40.7" end="473631:40:44.16">, B</span><span style="S12" begin="473631:40:40.9" end="473631:40:44.16">, C</span><span style="S12" begin="473631:40:41.1" end="473631:40:44.16">, D</span><span style="S12" begin="473631:40:41.3" end="473631:40:44.16">, E</span><span style="S12" begin="473631:40:41.5" end="473631:40:44.16">, F</span><span style="S12" begin="473631:40:41.7" end="473631:40:44.16">, G</span><span style="S12" begin="473631:40:41.9" end="473631:40:44.16">, A</span><span style="S12" begin="473631:40:42.1" end="473631:40:44.16">, B</span><span style="S11" begin="473631:40:42.3" end="473631:40:44.16">, C</span><span style="S11" begin="473631:40:42.5" end="473631:40:44.16">, B</span><span style="S11" begin="473631:40:42.7" end="473631:40:44.16">, A</span><span style="S11" begin="473631:40:42.9" end="473631:40:44.16">, G</span><span style="S11" begin="473631:40:43.1" end="473631:40:44.16">, F</span><span style="S11" begin="473631:40:43.3" end="473631:40:44.16">, E</span><span style="S11" begin="473631:40:43.5" end="473631:40:44.16">, D</span><span style="S11" begin="473631:40:43.7" end="473631:40:44.16">, C</span><span style="S11" begin="473631:40:43.9" end="473631:40:44.16">, B</span><span style="S11" begin="473631:40:44.1" end="473631:40:44.16">, A</span></p>
		</div>
	</body>
</tt>
@pjcherriman
Copy link
Author

pjcherriman commented Jan 16, 2024

I had seen a similar linegap issue on a TV using dash.js for playback

IMG_20240116_110504

It seems the issues is the TV (like many TVs) uses a devicePixelRatio of 1.5

@poolec
Copy link

poolec commented Mar 26, 2024

This problem, at least as far as the line gap is concerned, appears to be specific to the case where window.devicePixelRatio is greater than 1 (i.e. where the graphics are rendered at a higher resolution than the co-ordinate system). This is fairly common on TVs and mobiles and indeed in desktop browsers where high resolution displays are used.

Having investigated this further, I believe the problem arises because the implementation of fillLineGap pads each line to an integer vertical co-ordinate in the CSS pixel co-ordinate space. When the device is using a higher resolution for rendering, this does not necessarily align with a device pixel boundary. If that happens, then when the browser draws the background boxes and the positions are not integer positions, gaps can arise e.g. due to independent rounding of positions and sizes in the rendering process. (Overlaps may also occur but would not be apparent with the opaque backgrounds that we use).

I believe there is a fairly straightforward fix to this by rounding to device pixel boundaries. e.g. instead of applying Math.round in the applyFillLineGap function, use a function such as this:

function roundToDevicePixel(position) {
        return Math.round(position * window.devicePixelRatio)/window.devicePixelRatio;
}

@nigelmegitt
Copy link
Contributor

Worth checking against this test in particular:

This test uses background colours with around 50% opacity, so if there are overlapping background boxes then they look like darker lines at the overlap locations. If this fix doesn't break that test, I expect that it's probably fine. Obviously should pass all the other tests too.

@poolec
Copy link

poolec commented Mar 27, 2024

I tried that test. At 1.0 devicePixelRatio it works fine. At other devicePixelRatios, the generated PNG always comes out the same size so is not representative of the browser display. With or without my proposed fix, the generated image shows gaps.

The generated HTML shows no overlaps when viewed on the same browser at the same devicePixelRatio. I do still see a gap in one devicePixelRatio, even with the proposed fix, so will dig into that one a bit further.

@poolec
Copy link

poolec commented Mar 28, 2024

I've now done some more detailed investigation using this test.
What I see in Chrome can be summarised as follows:

  • When window.devicePixelRatio is 1.0, getBoundingClientRect returns values that are precise multiples of 1/64 pixel
  • When devicePixelRatio is 1.0, calculated padding values are precise multiples of 1/64 pixel and once padded, the resulting span has an integer vertical size
  • When devicePixelRatio is 1.25, getBoundingClientRect returns values that are close to multiples of 0.2 pixels but +/- something of the order of 0.00001
  • When devicePixelRatio is 1.25, calculated padding values are close to multiples of 0.2 pixels but +/- something of the order of 0.00001
  • When the bottom padding value is even a tiny bit less than the nearest multiple of 0.2 pixels, the background is rendered with a gap. If the bottom padding is a tiny bit more than a multiple of 0.2 pixels, there is no gap. For example: 2.799999237061 gives a cap. 2.8 does not.
  • When a gap is visible, the new span height returned by getBoundingClientRect after padding has been applied is ~1/64 pixels smaller than expected

The 1/64 figure I am seeing correlates with the fixed point representation with 6 fractional bits that appears to be used within Blink (and came originally from WebKit). It appears that some calculations within the Chrome browser are rounding down to the nearest 1/64th of a pixel in cases where rounding to nearest might appear more appropriate.

I also observe that whilst 1.25 (the devicePixelRatio I tested) can be precisely represented in floating point, 1/1.25 (0.8), and multiples of it, cannot. Thus I wonder whether some loss of precision in the layout calculations is contributing to the problem.

Based on these new observations, I have just tested a different workaround in which I add 0.0001 to each calculated bottom padding value in the implementation of applyFillLineGap so as to prevent the calculated padding values ever ending up slightly less than the 'expected' value.

Initial results suggest that the problem then does not arise (nor is there any overlap) but more tests would be needed to confirm.

This is, of course, not a very satisfactory fix for imscJS.

@nigelmegitt
Copy link
Contributor

@JibberJim did some work on this before to make it work with devicePixelRatio != 1 on the BBC fork. I did a diff between the BBC fork and the main repo and could not see any significant differences that would explain why it might work in one context but not in another.

@poolec
Copy link

poolec commented May 23, 2024

Having now investigated this issue further, it's clear that different browsers are showing different rounding behaviours when rendering subtitles on displays where devicePixelRatio is not 1.0. This causes gaps between lines at certain vertical positions. In some cases, there are even gaps between adjacent spans.

I do not believe there are any concrete requirements for the user agent covering rounding behaviour in such cases. As such it can't confidently be argued that it is a user agent problem. In my mind, it therefore falls to imscJS to do its best to avoid these gaps being visible to the end user across the range of browsers in use. (Separately, there may be a case for making the specs more precise.)

To my mind, the best solution would be to introduce a small (~1 pixel) overlap in the text backgrounds in the case where (i) fillLineGap is enabled, (ii) the background colours of adjacent lines are both opaque and (iii) devicePixelRatio is not 1.0. This would not prevent gaps in cases where semi-transparent backgrounds are used but in that case, there is a risk of creating visible overlaps instead which would probably look just as bad. It would address the problem fully for the opaque background case. It could be achieved by an additional 1 pixel of bottom padding and -1 pixel of bottom margin.

A similar approach could be taken between adjacent spans to eliminate vertical lines appearing between words.

It's possible that an overlap of less than 1 pixel may address the problems observed. Maybe 0.5 pixel or 1/devicePixelRatio or 0.5/devicePixelRatio might be sufficient. However, I'm not sure it is worth trying to find the absolute minimum that would solve the problems seen today since (i) future devices might be affected in different ways and (ii) the visible difference would be negligible.

This approach would not result in any text characters being clipped in the same way they would if, for example, line spacing were artificially reduced.

I plan to prototype this properly when I have time, but in the mean time, I'd welcome any comments or suggestions for improvement.

@palemieux
Copy link
Contributor

Thanks for all the sleuthing!

From an imscJS main branch perspective, I feel that the energy spent chasing UA shortcomings could be better spent either

  • relentlessly bugging UA folks to support missing CSS features so that imscJS and others do not have to implement fragile workarounds; or
  • fixing the shortcomings directly in the UA code.

For example, inline-sizing would make implementing fillLineGap less fragile.

[ed.: I do think imscJS needs workarounds for critical features, but there is diminishing returns in addressing all idiosyncrasies.]

@poolec
Copy link

poolec commented May 23, 2024

I understand the sentiment. However, the problem with that approach is that it really doesn't address the TV market. TVs invariably have browsers that are some way behind the cutting edge of desktop browsers (typically 2 years or more), very rarely receive updates and are infrequently replaced by their users.

As a content provider wishing to ensure our viewers get a great experience with subtitles on their TVs, we have to work with what is out there, much as we might wish it to be better. Since the majority of TV viewing is on a TV, and we're seeing the problems described here on many of them, I think it is entirely appropriate that imscJS acknowledge that reality and do what's reasonably possible to address it in the interest of subtitle users, provided that the fix doesn't break something else.

I don't want to waste time on a fix for this if you would reject it on principle. However, I think it could be written in a fairly self contained way and be made configurable. Would that be acceptable?

@palemieux
Copy link
Contributor

@poolec Can you share the compatibility requirements for those TV browsers? In particular, what is the lowest ECMA Script version support? The idea is to figure out a strategy for legacy support, esp. in light of the ongoing work on imscJS 2.0.

@poolec
Copy link

poolec commented May 24, 2024

Sure. They're supporting either HbbTV 2.0.3 or, for very recent models, HbbTV 2.0.4.
HbbTV 2.0.3 references the CTA WAVE Web Media API Snapshot 2018
HbbTV 2.0.4 references the CTA WAVE Web Media API Snapshot 2021
In terms of ECMAScript, they reference ECMAScript 6 and ECMAScript 2021 respectively, albeit with some exceptions.

@nigelmegitt
Copy link
Contributor

Hypothesis: when devicePixelRatio is not 1 the UA is incorrectly calculating the bottom or right edges of areas, because, in general:

round(origin * devicePixelRatio) + round(extent * devicePixelRatio) != round((origin + extent) * devicePixelRatio)

In other words, the theory is that the UA is using the wrong algorithm to calculate the edges in scaled views.

I put together a codepen to discover how frequently this might arise, at https://codepen.io/nigelmegitt/full/pomKoww - if this theory is correct, there are a lot of opportunities to generate 1px gaps or overlaps when devicePixelRatio is not 1.

Just to emphasise, it's a hypothesis only at this point.

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

No branches or pull requests

4 participants