-
Notifications
You must be signed in to change notification settings - Fork 31
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
Comments
This problem, at least as far as the line gap is concerned, appears to be specific to the case where Having investigated this further, I believe the problem arises because the implementation of I believe there is a fairly straightforward fix to this by rounding to device pixel boundaries. e.g. instead of applying
|
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. |
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. |
I've now done some more detailed investigation using this test.
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. |
@JibberJim did some work on this before to make it work with |
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. |
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
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.] |
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? |
@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. |
Sure. They're supporting either HbbTV 2.0.3 or, for very recent models, HbbTV 2.0.4. |
Hypothesis: when
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 Just to emphasise, it's a hypothesis only at this point. |
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.
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.
On the firefox image, the linegap can be seen between the two musical notes.
The TTML similar to this
The text was updated successfully, but these errors were encountered: