ebu / dash.js

A reference client implementation for the playback of MPEG DASH via Javascript and compliant browsers.
Other
11 stars 3 forks source link

linePadding not correct (test78) #36

Closed frans-ebu closed 9 years ago

frans-ebu commented 9 years ago

I see two issues with test78 rendering:

The linepadding should be applied as per page 50-51 in https://tech.ebu.ch/docs/tech/tech3380.pdf

SoleneChiche commented 9 years ago

@nigelmegitt: By inheritance, all the spans of the linePadding example in tech3380 are line padded, not just at the line beginning and ending. Could you help on the approach to adopt? I can see that your BBC implementation is correctly handling this particular case and I'm out of idea for now...

My structure is the following:

tt: Object
  body: Object
    div: Object
      p: Array[5]
        0: Object
          span: "Some "
          span@style: "bgBlack"
        1: Object
          span: "centered "
          span@style: "yellowText bgBlack"
        2: Object
          span: "text"
          span@style: "bgBlack"
        3: Object
          br: null
        4: Object
          span: "on two lines (no linePadding)."
          span@style: "bgBlack"
        length: 5
      p@begin: "00:00:00.000"
      p@end: "00:00:05.000"
      p@region: "region2"
      p@style: "withoutLinePadding"
      p@xml:id: "sub0"

If there is a span and that some linePadding was specified somewhere before (body, div, p), it will specify it for the span too, so for each and every span...

nigelmegitt commented 9 years ago

Hi @SoleneBuet I'm not sure exactly what the problem is with this issue - is it that you want a way in HTML+CSS to apply the line padding only to the edges of the line areas and not all the spans that may be in each line area?

for example, using the character ◻︎ to indicate background space created by linePadding, the effect you want is:

◻︎Some centered text◻︎ ◻︎on two lines◻︎

and you want to avoid:

◻︎Some ◻︎◻︎centered ◻︎◻︎text◻︎ ◻︎on two lines◻︎

I'm not certain what the correct approach is, but I think it must involve wrapping the lines in another span in the HTML, that can have the edge effect applied, perhaps using the box-decoration-break: clone; style to ensure that it applies regardless of how many lines the content ends up being broken into.

Does that help at all?

I think I sent you an example HTML document with this approach a while ago, but maybe it doesn't cover the full complexity that you need to?

SoleneChiche commented 9 years ago

Thank you Nigel, that is currently what I'm trying to do (wrap them in an external span) to avoid this. This CSS property seems very interesting in case of breaks (if it does wrap for example), I for sure will try it out.

frans-ebu commented 9 years ago

Tnx.

As discussed, the open issue is that if the line consists of multiple spans with diffierent coloured backgrounds for the start and end of the line, the algorithm will use one of the two colours for the spans. Not sure how/if this is fixable...

SoleneChiche commented 9 years ago

Yes, I think that the current way it is implemented, it won't be able to achieve that...

SoleneChiche commented 9 years ago

Maybe we should split this one. The original issue is corrected, but I'll open a new one concerning the different background color error.

frans-ebu commented 9 years ago

Let's leave this as a corner case we don't support for now.

I guess theoretically we could separate in the line in two parts, a starting span with left padding and a certain background colour and an ending one with a different background colour. But this is nice-to-have in future. Not a prio.

SoleneChiche commented 9 years ago

Yes our messages crossed. Can you now test the 78 to make sure and so we can close this one?

nigelmegitt commented 9 years ago

@frans-ebu if you split into left padding and right padding with separate spans then you'll get the wrong effect if there's a line break mid-span.

SoleneChiche commented 9 years ago

That was my guess indeed...

frans-ebu commented 9 years ago

@nigelmegitt But in theory you could detect the line break and add spans as needed, right?

Not saying that that is very elegant, but at least it could be solved?

nigelmegitt commented 9 years ago

@frans-ebu Not sure. I don't know of a technique for detecting line breaks.

SoleneChiche commented 9 years ago

Detecting them is not complicated (I'm already using an array of their indices) so I know where to put the wrapping spans. The thing is that we would have to split padding-right and padding left. No more wrapping anymore. Or to put empty span with a padding-right at the beginning and empty span with a padding left at the end. But as Nigel said, this won't work together with wrapOption

nigelmegitt commented 9 years ago

I meant detecting the post-layout line breaks. You can hand over a span to the layout engine and it might generate any number of lines without exposing the laid-out line areas back to the js. Or at least that's my understanding.

SoleneChiche commented 9 years ago

@frans-ebu can you tell me if it ok to close?

frans-ebu commented 9 years ago

Yes, we close this one as we now have the background colour part in #39